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

Support a way to ignore found issues in the future. #195

Closed
TheRealAgentK opened this issue Sep 15, 2016 · 14 comments
Closed

Support a way to ignore found issues in the future. #195

TheRealAgentK opened this issue Sep 15, 2016 · 14 comments
Assignees

Comments

@TheRealAgentK
Copy link
Collaborator

Let's say I in general want to run the rule that checks for missing default values in non-required function arguments.

But I might have a few custom instances for which I don't want this rule to be applied - maybe because it's been taken care of in the code. So we'd want to exclude a certain function from triggering it.

As far as I can see there's no way to currently do that. Is there any option to implement something like this with Annotations, similarly to how Java linters do offer that (for instance in Android linting). I'd annotate a function so that it ignores a specific rule for future runs.

@bardware
Copy link

I know how an empty log file in the search for errors feels but I believe it's deceptive to disable checks.
If you for instance log CFLint output as JSON and import in somwehere you might exclude results based on the data provided. filename and function name are output in most cases.
If you look at the raw output you would still see there's an issue - whether it's bad/important or not.

@TheRealAgentK
Copy link
Collaborator Author

Sure, I could build all sorts of solutions on top of the cflint output and manually filtering out results I don't want.

But having a way to annotate parts of ones code with instructions to not check and apply certain rules is really the norm. The fact that you find it deceptive to disable checks is appreciated, but you wouldn't have to use that feature if you don't like it. The option should be there nevertheless.

@ryaneberly
Copy link
Contributor

@TheRealAgentK , I hear you. We haven't supported code annotations yet. But you can accomplish some of this with the cflintexclude.json file
https://github.com/cflint/CFLint/wiki/Are-There-Any-Recipes-Out-There%3F

You can filter out codes by file, function name, and line number.

ryaneberly added a commit that referenced this issue Oct 15, 2016
Added access to the tokenstream in the Context.
ryaneberly added a commit that referenced this issue Oct 15, 2016
Added support for 
//cflint ignore:line

and 
//cflint ignore:MISSING_VAR
@ryaneberly
Copy link
Contributor

Added support for
//cflint ignore:line
which suppresses all messages for that line.

and
//cflint ignore:MISSING_VAR
which suppresses a comma separated list of message codes for that line.

@ryaneberly
Copy link
Contributor

What syntax would you suggest for function annotations?

something like:
/*
@CFLintIgnore MISSING_VAR
*/
function somefunction(){
myVar = 123;
}

@TheRealAgentK
Copy link
Collaborator Author

Yes, I like that.

In Android for instance, it'd be:
...
@SuppressLint("NewApi")
public void onBackStackChanged() { ... }
...

This syntax would obviously not work in CF, so it'd have to be in a comment as you've shown it.

One question: What would the syntax look like if I wanted to suppress multiple lint rules for a function?

In Android that'd work via:

@SuppressLint({"NewApi","StringFormatInvalid"}), but I find this syntax rather clunky and convoluted

Going with your example, I'd like to be able to just use a comma-separated list:

@CFLintIgnore MISSING_VAR,SOMETHINGELSE,ANOTHERTHINGTOIGNORE

@ryaneberly
Copy link
Contributor

I like it.

ryaneberly added a commit that referenced this issue Oct 22, 2016
Add support for @CFLintIgnore to component and function declarations.

Plus some cleanup, and better support for nested contexts.
ryaneberly added a commit that referenced this issue Oct 22, 2016
Add support for @CFLintIgnore to cfcomponent and cffunction tags.

cleanup bugs - all tests passing.
@ryaneberly
Copy link
Contributor

Support added at the line level in cfscript:
someVar = 123; // cflint ignore:line

at the component and function level in cfscript:

    /*
    other comments....
    @CFLintIgnore SOMETHINGELSE,MISSING_VAR,ANOTHERTHINGTOIGNORE
    */
         component{
      ......

at the component and function level in CFML:

       <!---
        other comments....
       @CFLintIgnore SOMETHINGELSE,MISSING_VAR,ANOTHERTHINGTOIGNORE
       --->
         <cfcomponent>
          .....

@TheRealAgentK
Copy link
Collaborator Author

TheRealAgentK commented Nov 23, 2016

Is there any support for lines in normal tag-based code outside ignoring a whole function or a whole component?

To properly explain what I mean:

I might have a sequence of CFML tag-based code. In one of the tags, I know there's an issue. So, it'd be good to be able to do:

<cfsomething>
<cfsomethingelse>
<!--- cflint ignore:RULE1,RULE2 --->
<cfsomethingWithAnIssueIWantToIgnore>
<cfmorestuff>

What the above pseudo code should do is then ignore RULE1,RULE2 in <cfsomethingWithAnIssueIWantToIgnore>

@ryaneberly
Copy link
Contributor

Yes @TheRealAgentK ,

only at the component and function level in CFML:

       <!---
       	other comments....
       @CFLintIgnore SOMETHINGELSE,MISSING_VAR,ANOTHERTHINGTOIGNORE
       --->
         <cfcomponent>
          .....

I think that's working. There's a test here:
https://github.com/cflint/CFLint/blob/cc45d5646dca6dd1a3a89854bead5b60bc6eebbd/src/test/resources/com/cflint/tests/Ignores/ignoreCFMLComponent2.cfc

@ryaneberly
Copy link
Contributor

the project is sadly in need of some documentation....

ryaneberly added a commit that referenced this issue Nov 25, 2016
support CFML comment ignores at all levels, not just <cfcomponent> and
<cffunction>
TheRealAgentK added a commit that referenced this issue Jun 7, 2017
Added testcase for #195 (ignoring rules) not working
@TheRealAgentK TheRealAgentK reopened this Jun 7, 2017
@TheRealAgentK
Copy link
Collaborator Author

The current mechanism doesn't seem to work across all rules.

In my case I can't get CFQUERYPARAM_REQ to be ignored at all. Added test case demonstrating the issue to dev (c88b366).

I'm not sure if there are any other places I could possibly put the @CFLintIgnore annotation :)

I noticed that all the test cases were for MISSING_VAR only - is there some dependency on a certain category of rules in the implementation of this?

@ryaneberly ryaneberly self-assigned this Jun 10, 2017
TheRealAgentK added a commit to TheRealAgentK/CFLint that referenced this issue Jun 19, 2017
ryaneberly pushed a commit that referenced this issue Jun 19, 2017
@ryaneberly
Copy link
Contributor

Is this one all set now?

@TheRealAgentK
Copy link
Collaborator Author

I think from the reoccured issue of CFQUERYPARAM_REQ point-of-view, yes!

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

No branches or pull requests

3 participants