Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update missing-owner-check lint to ignore if the key of the account is compared #73

Merged
merged 3 commits into from
Feb 8, 2024

Conversation

S3v3ru5
Copy link
Contributor

@S3v3ru5 S3v3ru5 commented Jan 30, 2024

The PR updates the lint to check if key of account is compared against some value. if the key is compared then the account is ignored by the lint and is not reported.

The update to the lint is as follows:

Given list of account_expr where each account_expr returns AccountInfo

  1. For each account_expr
  2. Walk through each expression in the body of the function
  3. Check if the expression is a comparison: == or !=
  4. Check if lhs or rhs of the comparison accesses the key on the account_expr
    • The key could be accessed using .key() for Anchor and .key for Solana AccountInfo.
    • Check if the expression is a method call to .key() or is a field access expression accessing .key.

The PR also makes some of the functions generic and removes redundant code.

Base automatically changed from fix-missing-owner to master February 7, 2024 17:37
@S3v3ru5 S3v3ru5 force-pushed the fix-include-key-checks branch from 736a4d3 to 236b611 Compare February 7, 2024 18:01
Copy link
Member

@smoelius smoelius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have one style nit. I think(?) before this PR the functions were topologically sorted from "callers" to "callees". Would it possible to preserve that?

So, for example, could compares_key appear before expr_accesses_key which appears before uses_given_field?

(Apologies if there is a cycle I am not seeing.)

@S3v3ru5
Copy link
Contributor Author

S3v3ru5 commented Feb 8, 2024

Update the lint. I wondered why the code was easy to follow. Will be sure to follow the style.

@smoelius
Copy link
Member

smoelius commented Feb 8, 2024

Beautiful.

@smoelius smoelius added this pull request to the merge queue Feb 8, 2024
Merged via the queue into master with commit 9cf182a Feb 8, 2024
3 checks passed
@smoelius smoelius deleted the fix-include-key-checks branch February 8, 2024 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants