Skip to content

Conversation

max-schaefer
Copy link
Contributor

These queries are currently run by default, but don't have their results displayed.

Looking through results on LGTM.com, they are either false positives (e.g., BitwiseSignCheck which flags many perfectly harmless operations and CompareIdenticalValues which mostly flags NaN checks) or harmless results that developers are unlikely to care about (e.g., EmptyArrayInit or MisspelledIdentifier).

With this PR, the only queries that are still run but not displayed are security queries, where different considerations may apply.

@max-schaefer max-schaefer requested a review from a team as a code owner May 19, 2020 11:00
@max-schaefer
Copy link
Contributor Author

Obviously, feel free to speak up if there are any queries in here that you think are worth keeping.

Two of these queries have associated CWEs; we can keep them on the strength of that, but I think it would be a stretch to call them security queries.

esbena
esbena previously approved these changes May 19, 2020
@max-schaefer max-schaefer changed the title Lower precision for a number of queries. JavaScript: Lower precision for a number of queries. May 19, 2020
Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

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

Agree with the choice of queries, just a minor nit in the change note.

@@ -23,6 +23,29 @@
| Expression has no effect (`js/useless-expression`) | Less results | This query no longer flags an expression when that expression is the only content of the containing file. |
| Unknown directive (`js/unknown-directive`) | Less results | This query no longer flags directives generated by the Babel compiler. |

The following low-precision queries are no longer run by default on LGTM (their results already were not displayed previously):
Copy link
Contributor

Choose a reason for hiding this comment

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

Saying both "already" and "previously" seems a bit redundant.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed "previously".

These queries are currently run by default, but don't have their results displayed.

Looking through results on LGTM.com, they are either false positives (e.g., `BitwiseSignCheck` which flags many perfectly harmless operations and `CompareIdenticalValues` which mostly flags NaN checks) or harmless results that developers are unlikely to care about (e.g., `EmptyArrayInit` or `MisspelledIdentifier`).

With this PR, the only queries that are still run but not displayed are security queries, where different considerations may apply.
@max-schaefer
Copy link
Contributor Author

@esbena, can I get a re-approval?

@semmle-qlci semmle-qlci merged commit 26dfca8 into github:master May 19, 2020
@max-schaefer max-schaefer deleted the cull-boring-queries branch May 26, 2020 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants