Skip to content

JavaScript: Introduce new query UnclearOperatorPrecedence. #260

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

Merged
merged 2 commits into from
Oct 3, 2018

Conversation

xiemaisi
Copy link

@xiemaisi xiemaisi commented Oct 1, 2018

When using bitwise operators, it's easy to write code like x & 4 !== 0 without realising that this is interpreted as x & (4 !== 0) and not (x & 4) !== 0. My favourite example of this comes from an old version of Angular.js, but other examples abound.

The second commit adds a query for flagging such nestings between bitwise operators and relational operators, which reuses some logic from the whitespace-contradicts-precedence query pulled out in the first commit. I improved that query to take advantage of per-token location while I was at it.

Performance looks fine (it's of course an entirely syntactic query), and there are many decent results. I've made it a recommendation since smart people use this sort of idiom on purpose.

@xiemaisi xiemaisi added the JS label Oct 1, 2018
@xiemaisi xiemaisi requested a review from a team as a code owner October 1, 2018 16:25
@ghost ghost self-assigned this Oct 2, 2018
@xiemaisi xiemaisi force-pushed the js/confusing-precedence branch from 8b05e26 to 7683684 Compare October 2, 2018 07:46
Copy link

@ghost ghost left a comment

Choose a reason for hiding this comment

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

LGTM

(I made GitHub resolve the change-note conflict automatically, it looks like it did a rebase)

Copy link
Contributor

@mchammer01 mchammer01 left a comment

Choose a reason for hiding this comment

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

Editorial review completed, nothing to report.
LGTM.

@semmle-qlci semmle-qlci merged commit e9adc63 into github:master Oct 3, 2018
@xiemaisi xiemaisi deleted the js/confusing-precedence branch October 4, 2018 09:35
@kamarcum kamarcum unassigned ghost and mchammer01 Apr 28, 2020
aibaars pushed a commit that referenced this pull request Oct 14, 2021
smowton added a commit to smowton/codeql that referenced this pull request Apr 16, 2022
…-substitution-done

Note type substitution TODO done
MathiasVP pushed a commit to MathiasVP/ql that referenced this pull request Aug 10, 2025
…alization

Powershell Unsafe Deserialize query
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.

3 participants