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.

Sign up for GitHub

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

Jump to bottom

fix lint regression in non_upper_case_globals #110575

Merged
merged 1 commit into from
Apr 20, 2023
Merged

fix lint regression in non_upper_case_globals #110575

merged 1 commit into from
Apr 20, 2023

Conversation

Ezrashaw
Copy link
Contributor

Fixes #110573

The issue also exists for inherent associated types (where I copied my impl from). EarlyContext is more involved to fix in this way, so I'll leave it be for now (note it's unstable so that's not urgent).

r? @compiler-errors

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Apr 20, 2023
compiler-errors
compiler-errors requested changes Apr 20, 2023
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hm.. I think this is a bit too complicated for what it's worth.

Can you just revert your original PR, and just check the associate const's container similarly to how method_context does to make sure that the NonSnakeCase lint only fires on inherent methods?

@Ezrashaw Ezrashaw force-pushed the fix-regression-110573 branch from a677d9e to 4766d44 Compare April 20, 2023 04:21
@Ezrashaw Ezrashaw changed the title fix: Cellify "last node" field on LateContext fix lint regression in non_upper_case_globals Apr 20, 2023
@Ezrashaw
Copy link
Contributor Author

@compiler-errors I reverted and re-implemented. I haven't fixed it for inherent associated types, would you like that in this PR or a followup? (note that it's an early lint so we'd need to convert it to LateContext)

compiler-errors
compiler-errors approved these changes Apr 20, 2023
View reviewed changes
Copy link
Member

@compiler-errors compiler-errors left a comment

Choose a reason for hiding this comment

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

please squash this into one commit, and preferably (but I guess not required) fix the NonCamelCaseTypes lint for associated types too?

tests/ui/lint/issue-110573.rs Outdated Show resolved Hide resolved
@Ezrashaw Ezrashaw force-pushed the fix-regression-110573 branch from 4766d44 to 8cad917 Compare April 20, 2023 05:04
@Ezrashaw
Copy link
Contributor Author

Ezrashaw commented Apr 20, 2023 •
edited
Loading

@compiler-errors I've implemented your review comments. The inherent associated types part is too complex to do right now, unrelated parts of the lint stop working when I tried to make it a late lint. It is and unstable feature so we might just leave it as is for now?

Also, sorry that this is taking so long; I only have access to my laptop right now.

@compiler-errors
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 20, 2023

📌 Commit 8cad917 has been approved by compiler-errors

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 20, 2023
@JohnTitor JohnTitor mentioned this pull request Apr 20, 2023
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Apr 20, 2023
@bors
Auto merge of rust-lang#110585 - JohnTitor:rollup-gfffoiv, r=JohnTitor
13fc33e 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#102341 (Implement `Neg` for signed non-zero integers.)
 - rust-lang#110424 (Spelling misc)
 - rust-lang#110448 (cmp doc examples improvements)
 - rust-lang#110516 (bootstrap: Update linux-raw-sys to 0.3.2)
 - rust-lang#110548 (Make `impl Debug for Span` not panic on not having session globals.)
 - rust-lang#110554 (`deny(unsafe_op_in_unsafe_fn)` in `rustc_data_structures`)
 - rust-lang#110575 (fix lint regression in `non_upper_case_globals`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a2826dc into rust-lang:master Apr 20, 2023
@rustbot rustbot added this to the 1.71.0 milestone Apr 20, 2023
@Ezrashaw Ezrashaw deleted the fix-regression-110573 branch April 26, 2023 09:03
@Ezrashaw Ezrashaw restored the fix-regression-110573 branch May 3, 2024 08:11
@Ezrashaw Ezrashaw deleted the fix-regression-110573 branch May 13, 2024 10:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@compiler-errors compiler-errors compiler-errors approved these changes

Assignees

@compiler-errors compiler-errors

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.71.0
Development

Successfully merging this pull request may close these issues.

Regression in non_upper_case_globals lint level attribute on associated item
4 participants
@Ezrashaw @compiler-errors @bors @rustbot