Skip to content
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

Prevent replace_consts lint within match patterns #4977

Merged

Conversation

krishna-veerareddy
Copy link
Contributor

Currently replace_consts lint applies within match patterns but the suggestion is incorrect as function calls are disallowed in them. To fix this we prevent the lint from firing within patterns.

Fixes #4969

changelog: Fix false positive in replace_consts lint

Currently `replace_consts` lint applies within match patterns but
the suggestion is incorrect as function calls are disallowed in
them. To fix this we prevent the lint from firing within patterns.
@JohnTitor JohnTitor added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 1, 2020
Copy link
Member

@phansch phansch left a comment

Choose a reason for hiding this comment

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

r=me with test added

std::i8::MIN => -1,
1..=std::i8::MAX => 1,
_ => 0
};
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a test with if let as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I didn't think about that use case. Done.

@phansch
Copy link
Member

phansch commented Jan 2, 2020

@bors r+ thanks!

@bors
Copy link
Collaborator

bors commented Jan 2, 2020

📌 Commit 8b36196 has been approved by phansch

@bors
Copy link
Collaborator

bors commented Jan 2, 2020

⌛ Testing commit 8b36196 with merge 5b710ee...

bors added a commit that referenced this pull request Jan 2, 2020
…, r=phansch

Prevent `replace_consts` lint within match patterns

Currently `replace_consts` lint applies within match patterns but the suggestion is incorrect as function calls are disallowed in them. To fix this we prevent the lint from firing within patterns.

Fixes #4969

changelog: Fix false positive in `replace_consts` lint
@bors
Copy link
Collaborator

bors commented Jan 2, 2020

☀️ Test successful - checks-travis, status-appveyor
Approved by: phansch
Pushing 5b710ee to master...

@bors bors merged commit 8b36196 into rust-lang:master Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

replace_consts false positive when using integer constant as part of range in match
4 participants