Skip to content
This repository has been archived by the owner on Jul 20, 2018. It is now read-only.

Fix false positives #18

Open
joshbetz opened this issue Oct 11, 2013 · 3 comments
Open

Fix false positives #18

joshbetz opened this issue Oct 11, 2013 · 3 comments
Labels

Comments

@joshbetz
Copy link
Contributor

from @nickdaugherty:

  • False positives with stuff in comments – like /* file ( http://something.com/file.php ) */ – just explaining the file in question, not reading it
  • False positives for class methods that have the same name as native php functions, like delete()
  • False positives for ‘Possible database table creation’ – files named upgrade_ (or maybe it’s files w/ events in the name)
@natebot
Copy link
Contributor

natebot commented Jan 12, 2014

At Grist we are in a position that we could use the vip-scanner via wp-cli as a pre-commit script before commits.

Unfortunately there are two issues keeping us from doing this:

  1. Warnings make the scanner return a Failed status -- Our theme has lots of warnings that are accurately diagnosed but are actually benign and have been accepted by a manual review by VIP engineers.
  2. False positives like those noted above.

Firstly, shouldn't Warning results allow the scan to pass? Warnings are about possible dangers, not guaranteed problems right?

Secondly, since chasing down every condition that could lead to false positives and coding against them is impractical, couldn't developers silence certain errors or warnings in their environment so that they can ignore that noise when scanning? The Log Deprecated Notices plugin supports this feature, so maybe the same technique would work here? I'd like our developers to see that their code generates any new warnings - its hard to find the needle in the stack of 50+ existing false positives, etc.

Apologies if this is the wrong place to bring this up. We'd be happy to contribute to this feature if you agree it's valuable.

@joshbetz
Copy link
Contributor Author

couldn't developers silence certain errors or warnings in their environment so that they can ignore that noise

I think it would be better to fix the false positives.

@natebot
Copy link
Contributor

natebot commented Jan 13, 2014

In principle I agree. It just seems more of a practical matter to me though. We'll do our best to contribute against the false positives we're having.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants