rust-lang / rust-lang/rust-clippy
Support triggering `clippy::precedence` on every expression with more than one infix operator
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 13.5k
- Forks
- 2.2k
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 32
Description
What it does
Warn about all unparenthesised uses of multiple infix operators (aside from ., probably).
It should probably be an option affecting clippy::precedence to opt into this stricter behaviour.
Possible modes for that option:
- shifts (default, current behaviour)
- different, which would warn on any expression using more than one kind of infix e.g.
a + b - cwould warn buta + b + cwould not - all, which would warn on all expressions using more than one infix period:
a - b - cis probably useful to disambiguate in case one fails to remember that expression is LTR, or has just inlined the second factor; and even operations which are traditionally associative may not be in all context (floats)
It would probably make sense for this extension to only warn on operators with an associativity lower than as (possibly including it), operators with higher precedences are traditionally seen / interpreted somewhat differently and less problematic in their reading
Advantage
Requires clarification of evaluation order for all expressions in high-sensitivity codebases.
Currently clippy::precedence flags un-parenthesised mixings of a few operator classes (mostly bitshift with other operator classes). However even mixing operators within the same precedence class can be risky, and requiring order of operation clarification can surface those issues.
Drawbacks
A new option which increases maintenance burden?
Example
a + b - c
Could be written as:
(a + b) - c
Comparison with existing lints
No response
Additional Context
FreeBSD just published a CVE whose fix is:
- args->endp - args->begin_argv + consume);
+ args->endp - (args->begin_argv + consume));
The second example of CWE-783 is
public double calculateReturnOnInvestment(double currentValue, double initialInvestment) {
double returnROI = 0.0;
// calculate return on investment
returnROI = currentValue - initialInvestment / initialInvestment;
return returnROI;
}
so I would assume this is not the first occurrence of "innocuous" precedence being critically incorrect.
Although the curated examples seem to mostly be issues with the assignment operator in C/C++.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reading the existing clippy::precedence lint and compare its current behavior with the proposed shifts, different, and all modes. Define the option's scope and expected warnings from the a + b - c and a - b - c examples; done means the selected mode consistently controls which mixed infix expressions are reported.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- tooling
- Issue type
- Feature
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100