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

Negative Modifiers Functionality for mdreport #162

Merged
merged 3 commits into from
Apr 7, 2022

Conversation

Rhynorater
Copy link
Contributor

I created a feature in my private fork of surya that adds the -n parameter to the mdreport command. When this parameter is defined, it also lists the modifiers that are NOT present. This may help catch issues relating to missing modifiers.

image

In the above scenario, onlyMinters, onlyMasterMinter, checkWhitelist, are shown to be NOT present.

Note that the way this is implemented is by running:

    parser.visit(ast, {
      ModifierInvocation: function ModifierInvocation(node){
        if (modifiers.indexOf(node.name) == -1){
          modifiers.push(node.name);
        }
      },
      ModifierDefinition: function ModifierDefinition(node){
        if (modifiers.indexOf(node.name) == -1){
          modifiers.push(node.name);
        }
      }
    });

before the main call to parser.visit to enumerate all of the modifiers called in this file. All modifiers that are invoked AND all modifiers that are defined are evaluated and pushed into an array. That array is used to determine which modifiers are missing. If there is a better implementation for this, I'd love to know.

Thanks, hope others will find this useful as well.
Rhynorater

Copy link
Collaborator

@GNSPS GNSPS left a comment

Choose a reason for hiding this comment

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

LGTM! Thank you so much @Rhynorater !!! 🙏 🙌 I'm sorry for the delay!

@GNSPS GNSPS changed the base branch from master to develop April 7, 2022 01:27
@GNSPS GNSPS merged commit 6019438 into Consensys:develop Apr 7, 2022
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