Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Make error/warning emission configurable per rule #157

Closed
mahonnaise opened this Issue · 6 comments

3 participants

Jos Hirth Nicholas C. Zakas Robert Massa
Jos Hirth

Within rules you can emit errors or warnings. That's kinda nice for the person who wrote the rule, but it won't necessarily match a user's requirements (which depend on the project).

E.g. with some legacy project one specific rule might be a nice-to-know thing, but with a new project it could be stop-the-press (and punch the code monkey) thing.

While it's true that you could theoretically filter the results (per project), I kinda feel like this would be a big waste since everyone would have to replicate this functionality.

Nicholas C. Zakas
Owner

I'm having trouble envisioning how this would work. Right now, errors are those things that you must fix because it will cause an issue in the browser while warnings are more stylistic. Do you have something specific in mind?

Jos Hirth

Well, you usually want to be more lenient with some legacy project/system. You won't rewrite it. And you probably won't care if there are some IDs used throughout the stylesheets. However, you're somewhat interested in this particular rule (and maybe a dozen others) for the sake of trends. So, if it gets worse over time, you'll see this trend (eventually) and you can intervene if it's getting out of hand.

With a fresh project things look different. You might want to take a super strict route there. I plan to use around ~25 rules (~14 from the current set) of which all but two (statistical ones) should be critical. They are all things which you should (and can) fix. There won't be anything fuzzy. There won't be any suggestions.

Ahm... OK. There is one thing I should probably mention: From my point of view, every rule should do exactly one thing. That's why there won't be a need to differentiate between different types of output within one rule.

Now that I think about it, it would be more like stdout and stderr. Either a message or an error. So, basically... every rule can be set to undefined (disabled), "log", or "critical".

How do other static code analysis tools handle this?

Nicholas C. Zakas
Owner

Other tools, such as JSLint, don't have options for this. That's why I was asking if you had something specific in mind. Can you do some research and see if you can find anything that would fit?

Jos Hirth

JSLint is a bit of a special case, I'd say. There are no warnings and there also aren't any statistics. There also aren't any false positives. Crockford paid a lot attention to this detail. He only included checks which can 100% accurately identify one specific problem.

Technically, he kinda cheated. Many checks focus on specific conventions and they also imply that you restrict yourself to a more manageable subset of the language.

Basically, I want to use the same trick. A nice subset, naming conventions, and automated tests which enforce/verify both parts. Just like JSLint, it won't prove that everything is correct; it will "just" ensure that there isn't anything obviously wrong with it. Like some sort of instant code review in a can. That's the way I like it.

The only other static code analysis tool I used was FindBugs (Java), but that was 8 years (or so) ago. Its output was very noisy with usually more than 50-75% false positives. While it was kinda helpful, I didn't actually use it because it wasn't suitable for automation.

Yesterday's drive-by "research":

http://www.slideshare.net/sebastian_bergmann/continuous-integration-with-jenkins-ipcse11-8165064

Slide 15/22 3rd line of the output (from phpmd):

... has an overall complexity of 105 which is very high. The configured complexity threshold is 50.

So, there are at least some tools which allow you to configure some threshold.

Many lint tools seem to support annotations and/or comments to suppress messages per function/line.

I didn't find one which allows you to switch between warnings/errors per rule.

Hm. C/C++ compilers usually allow you to turn warnings into errors via some command line switch. This would also do the trick, if you can also set a threshold for the counting ones (this would also address those "how much is too much" issues).

Robert Massa

I'd love this feature too, and maybe will implement this later. Currently I'm writing an addition which makes it possible to treat warnings as errors, which solves our use-case for now.

Nicholas C. Zakas
Owner

Oops, must have forgot to mention the bug number in the commit message. This has been completed and will be in the next version. You can use --warnings=rule,rule and --errors=rule,rule to make this configuration on the command line.

Nicholas C. Zakas nzakas closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.