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

Add autofixable suggestion for unseparated integer literal suffixes #4401

Merged
merged 4 commits into from
Aug 20, 2019

Conversation

JJJollyjim
Copy link
Contributor

@JJJollyjim JJJollyjim commented Aug 17, 2019

changelog: Add autofixable suggestion for unseparated integer literal suffixes

Somewhat WIP, since I haven't been able to get this working when adding // run-rustfix to ui/literals.rs. I think the issue is that there are multiple suggestions operating on one numerical literal, and I'm not sure what the best approach is to work around that.

Thanks

@JJJollyjim JJJollyjim force-pushed the literal-separation-suggestion branch from ae6d1c8 to 0922682 Compare August 18, 2019 11:38
@flip1995
Copy link
Member

You could split up the test case in tests especially for this lint and the other tests. This would also help with #2038. ;)

@JJJollyjim
Copy link
Contributor Author

Okay, will do 👍

Was just checking that this is only a test issue and not something end users will notice

@JJJollyjim JJJollyjim force-pushed the literal-separation-suggestion branch from 0922682 to a388a0b Compare August 18, 2019 14:54
@JJJollyjim
Copy link
Contributor Author

No longer WIP, assuming this CI passes :)

@JJJollyjim JJJollyjim force-pushed the literal-separation-suggestion branch from a388a0b to 802a6d3 Compare August 18, 2019 14:59
Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

Thanks!

tests/ui/literals.stderr Outdated Show resolved Hide resolved
clippy_lints/src/misc_early.rs Outdated Show resolved Hide resolved
@JJJollyjim JJJollyjim force-pushed the literal-separation-suggestion branch from 35bfe1e to 4ee9d02 Compare August 19, 2019 02:20
@tesuji
Copy link
Contributor

tesuji commented Aug 19, 2019

@flip1995 Should this lint on constant literals only? I tried to improve this lint too
but this lint triggers on event assert! macro. :D:

@tesuji
Copy link
Contributor

tesuji commented Aug 19, 2019

An error message:

---- [ui] ui/filter_map_next.rs stdout ----
normalized stderr:
error: LIT: Lit { token: Lit { kind: Integer, symbol: 7, suffix: Some(u32) }, node: Int(7, Unsigned(u32)), span: Span { lo: BytePos(180), hi: BytePos(209), ctxt: #0 } }
  --> $DIR/filter_map_next.rs:7:5
   |
LL |     assert_eq!(element, Some(1));
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: `-D clippy::unseparated-literal-suffix` implied by `-D warnings`
help: SPAN: Span { lo: BytePos(180), hi: BytePos(209), ctxt: #0 }
   |
LL |     assert_eq!(element, Some(1_u32
   |

Edit: Because I removed these 2 lines:

if let Some(firstch) = src.chars().next();
if char::to_digit(firstch, 10).is_some();

@flip1995
Copy link
Member

There's probably something weird going with the macro expansion. I can't really tell what exactly, without seeing all of your changes.

Should this lint on constant literals only?

I'm not sure if I understand your question correctly. Literals are always constant, since they're always one of a literal string ("str"), a literal number (42, 3.14) or a literal boolean (true). (docs)

@flip1995
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 20, 2019

📌 Commit 370433f has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Aug 20, 2019

⌛ Testing commit 370433f with merge 835205b...

bors added a commit that referenced this pull request Aug 20, 2019
…p1995

Add autofixable suggestion for unseparated integer literal suffixes

changelog: Add autofixable suggestion for unseparated integer literal suffixes

Somewhat WIP, since I haven't been able to get this working when adding `// run-rustfix` to `ui/literals.rs`. I think the issue is that there are multiple suggestions operating on one numerical literal, and I'm not sure what the best approach is to work around that.

Thanks
@bors
Copy link
Collaborator

bors commented Aug 20, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing 835205b to master...

@bors bors merged commit 370433f into rust-lang:master Aug 20, 2019
bors added a commit that referenced this pull request Aug 26, 2019
Cleaner code for unsep literals

Continuing discussion in #4401 (comment)
changelog: none
r? @flip1995
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants