JSHint Style Ignores about grunt-search HOT 4 OPEN

Oh yeah, super bonus points if you can also include an option to automatically ignore text in comments. I haven't thought about this as much, but I feel like the same techniques can be applied.

Use case: clearly the comment here should not provide a match, even if I want to check the grunt file for other console.logs.

/* Plugin: Search
 * ------------------------------------------------------
 * [Docs](https://www.npmjs.com/package/grunt-search)
 *
 * Search for text in files and log it or fail the task. 
 */
      search: {
          // search for console.log statements
          console: {
              ...
          }
      }

from grunt-search.

Hey @jtfairbank, great ideas! Yeah I could see these being very useful - especially identifying string matches within comments. Very practical. Wonder if there's a pre-existing jshint ignore-comment parser out there?

But I think we're in the same boat: I'm going go to be extremely busy in the coming weeks and won't get time to look at this. I'd definitely welcome a PR though if and when you get time. As long as both formats are still accepted: the simple regexp pattern like now, or a jshint ignore comment [with a flag to indicate one of the other?] I'd be very much on board with this.

Great stuff.

from grunt-search.

@benkeen Sorry for the wall of text. This is actually quite a fun problem to think about, and I geek out about this stuff late at night. I'll try to get to it, lets just keep each other informed on this issue if either one of us has ideas or questions / starts work / wants feedback / etc.

JSHint Code / Reuse

I looked into the jshint source- they actually implement a full lexer and parser to understand the javascript code (at a language analysis level, not compiler / runtime level) so I don't think their code would be very useful. It's all open source though, the relevant files are:

Code

• Ignore's Options
• jshint Lexer - Linked to the function that parses comments. About 50 lines down is the ignore command parsing code. This seems very useful as a starting point to parse out inline ignore commands.

Tests

• Ignore Line / Start & End Syntax Parsing - These seem very helpful as a starting point to check that the parser correctly identifies inline ignore commands.
• Ignore Line
• Ignore Start & End
• Ignore Start & End Wrapping a Comment

Simple Regex vs. Ignore Comments

A bit confused by what you meant by:

As long as both formats are still accepted: the simple regexp pattern like now, or a jshint ignore comment [with a flag to indicate one of the other?] I'd be very much on board with this.

I think both can work at the same time. The simple regex (searchString option) decides what matches in the first place (or not). The ignore comments that add an additional layer of discretion which allow specific lines or sections of code to be marked as "ok" even if they match the regex pattern.

Additional Thoughts on the Implementation

Currently the following options are available:

1. Add custom per-line ignore command via the searchString option (as seen in my first comment).

2. Implement a function for the logCondition option that can include slightly more complex code and choose to ignore matches or not.

   For example, here you could limit the ignore's to a specific filetype, range of line numbers (skip the license comment on lines 1 - 10 of every file), or based on the match itself.

   Since this can be set per command, it can also be used to decide if / when to ignore certain lines (ie ignore some for "grunt-search:console", others for "grunt-search:window", and all for "grunt-search:something-weird".

Improved options include:

• An ignoreCommandString option similar to the searchString, which allows the user to define custom regex to identify lines that might match but should be ignored. The following example should have the same effect as my complex regex in the first comment.

  options: {
      searchString: "console.", // console.log, console.warn, etc
      ignoreString: "grunt-search:console ignore:line"
          // if the line also contains this, ignore it
  }
  

• An improved logCondition function which provides the full line's text for additional custom analysis. This can be set per-command (as noted above), so I don't think it needs to include the command name (ie "grunt-search:console"). However that might also be useful in deciding if / when to ignore certain lines.

• A full blown ignore-comment system. This could be done quite simply, or could be very complicated, depending on what functionality you want. For example: only consider ignore directives in comments or just anywhere as text, as described below.

Additional Thoughts on Detecting Comments

Something to consider is that your plugin can work for many programming / non-programming languages and file types, whereas jshint is javascript specific.

Honestly I typed a bunch of stuff out here, but in reality it is not possible to correctly parse comments for generic languages. There are too many edge cases, variations between comment systems, etc. My recommendation is to just do a regex on each line for the ignore command string and implement the functionality based on that. I've left my notes below since they're already typed up and may be interesting.

To do this, grunt-search should support custom comment string via an option, possibly scoped to a file type:

options: {
    applyDefaultCommentTypes: true, // overridden by commentStyles
    commentStyles: [
        {
            multilineStart: "/*",
            multilineEnd: "*/",
            singleLine: "//",
            fileTypes: [".js", ".c", ...]
        },
        {
            multilineStart: "<!--",
            multilineEnd: "-->",
            fileTypes: [".html". ".handlebars"]
        },

        // NOTE: How to best handle languages that allow multiple comment styles for each category?
        //       ".handlebars" is included above for html style comments, and below for both of their own style comments.
        //       Maybe have each entry be specific to a single language, specified in the key? "'.handlebars': { ... comment styles ...}"
        {
            multilineStart: ["<!--", "{{!", "{!--"],
            multilineEnd: ["-->", "}}", "--}}"],
            fileTypes: [".handlebars"]
        },
    ]
}

It may also want to include a set of sane defaults tied to common programming languages (/* ... */, // ..., <!-- ... -->, etc) which are included by default and chosen based on the file type. The options would override this, and if a file type isn't found in the options or defaults a warning is thrown, or we ignore trying to parse for comments.

Another issue is that we would be doing 'naive' lexing / parsing by using regexes or string searches on each line instead of reading them in symbolically.

Perhaps the best way to go would be to skip checking for comments all together and just say "if the directive is included anywhere on the line, including in the active code as a string, etc, then we will apply it". Personally I feel like this is the best approach: let's keep it simple for now.

from grunt-search.

Dammit... goodbye productivity tonight. ;)

from grunt-search.