Implement crate-level-only lints checking. #73300

crlf0710 · 2020-06-13T02:52:32Z

This implements a crate_level_only flag on lints, and when it is true, it becomes an error when user tries to specify this flag upon nodes other than crate node.

This also turns on this flag for all non_ascii_ident lints.

rust-highfive · 2020-06-13T02:52:35Z

Some changes occurred in diagnostic error codes

cc @GuillaumeGomez

rust-highfive · 2020-06-13T02:52:36Z

r? @davidtwco

(rust_highfive has picked a reviewer for you, use r? to override)

crlf0710 · 2020-06-13T03:04:08Z

r? @petrochenkov

Also opened #73301 to track the sub-optimal error reporting.
Zulip thread: https://rust-lang.zulipchat.com/#narrow/stream/182449-t-compiler.2Fhelp/topic/Questions.20about.20lint.20level.20checking.2E

ehuss · 2020-06-13T03:54:32Z

There are some other lints that are ignored in non-crate positions (like elided_lifetimes_in_paths, #71957), which causes confusion. I had a couple questions regarding that:

Would it make sense to use this check for those lints? Since it would be a breaking change, I suspect it might need to be a future-compatible lint, so maybe this won't work as-is for pre-existing lints?
Should those lints be fixed so that they work in any position, or is it acceptable that they only work at the crate level?

crlf0710 · 2020-06-13T05:36:42Z

oops, accidentally overwrote this branch with another, recovered.

crlf0710 · 2020-06-13T05:43:34Z

Re: breaking change, I'm open to downgrade this check to a warning if people think it's a better idea.
I intent to make this general and make it able to apply to other crate-level-only lints too.

I think the non-ascii-idents series of lints should be kept crate-level-only, otherwise will bring a lot of runtime-complexity here. I don't have any opinion on whether any other specific lint should be crate-level-only or not...

EDIT: Thinking more about it, maybe it's possible to convert the two checks here to lints themselves. "Meta lints" lol.

petrochenkov · 2020-06-13T12:56:53Z

This can use the unused_attributes lint instead of a hard error.

It's hard to diagnose this precisely enough for a hard error.
Some early lints can only use approximate nodes, for example, so you can silence them at item level, but not at expression level.
Some lints use crate node id only in some cases, and use more precise ids in other cases.

petrochenkov · 2020-06-13T12:58:14Z

It would be good to audit all lints and detect those using CRATE_NODE_ID as a node id.

UPD: it's ELIDED_LIFETIMES_IN_PATHS, UNUSED_DOC_COMMENTS, UNKNOWN_CRATE_TYPES, UNUSED_CRATE_DEPENDENCIES, ILL_FORMED_ATTRIBUTE_INPUT and MACRO_EXPANDED_MACRO_EXPORTS_ACCESSED_BY_ABSOLUTE_PATHS.

src/test/ui/lint/crate_level_only_lint.rs

crlf0710 · 2020-06-15T18:55:55Z

I reimplemented this check, however it turns out to be a bigger change than i expected. But i've got it finished anyway. Let me leave a few notes here:

This brings up the concept of invalid attributes - An invalid attribute is one attribute that is used but then rejected by rules. A few APIs was added within the librustc_ast part.
Specifying crate level lint at non crate level position makes the lint attribute an invalid attributes.
And the preexisting check - detecting that if a lint is first marked forbidden level, then user try to overrule that within the region - also makes the lint attribute an invalid attribute.
Invalid attributes gets a warning message emitted, then the attribute itself is ignored. So the pre-existing E0453 error gets downgraded to a warning.
We only emit warnings when we mark the attribute invalid, so we won't emit the same warning multiple times. thus fixing Errors reported in rustc_lint::levels::LintLevelsBuilder::push occurs multiple times #73301
I also updated the existing test cases output.

crlf0710 · 2020-06-15T18:59:11Z

I'm not sure what do i need to do to the lint list above ... Should i add the crate_level_only flag to these lints?

petrochenkov · 2020-06-16T16:17:58Z

Nooo, not one more infra for invalid attributes, we already have two of those...
(In src\librustc_passes\check_attr.rs and in src\librustc_parse\validate_attr.rs.)

The original version was almost ready for merging, it only needed changing the error into a lint and marking more lints with crate_level_only.

petrochenkov · 2020-06-16T16:20:59Z

I the new version is mostly intended to fix #73301, then perhaps it's better submitted as a separate PR.
(I haven't read it in detail yet, maybe in a couple of days.)

petrochenkov · 2020-06-17T22:24:18Z

Implementation looks good to me, except that I'd still prefer to reuse unused_attributes.
(Then it makes sense to reword the message as "... is ignored unless specified at crate level".)

In theory, new lints still require an RFC, even if nobody remembers about it nowadays.
I think being stricter here still makes sense to avoid growing stable surface provided by the compiler, and to avoid proliferation of fringe lints like this one.

crlf0710 · 2020-06-18T01:56:26Z

Sure, i've adjusted the code. Thanks!

petrochenkov · 2020-06-18T09:22:05Z

Thanks!
@bors r+

bors · 2020-06-18T09:22:07Z

📌 Commit f633dd3 has been approved by petrochenkov

…petrochenkov Implement crate-level-only lints checking. This implements a crate_level_only flag on lints, and when it is true, it becomes an error when user tries to specify this flag upon nodes other than crate node. This also turns on this flag for all non_ascii_ident lints.

@ghost

…arth Rollup of 13 pull requests Successful merges: - rust-lang#71568 (Document unsafety in slice/sort.rs) - rust-lang#72709 (`#[deny(unsafe_op_in_unsafe_fn)]` in liballoc) - rust-lang#73214 (Add asm!() support for hexagon) - rust-lang#73248 (save_analysis: improve handling of enum struct variant) - rust-lang#73257 (ty: projections in `transparent_newtype_field`) - rust-lang#73261 (Suggest `?Sized` when applicable for ADTs) - rust-lang#73300 (Implement crate-level-only lints checking.) - rust-lang#73334 (Note numeric literals that can never fit in an expected type) - rust-lang#73357 (Use `LocalDefId` for import IDs in trait map) - rust-lang#73364 (asm: Allow multiple template string arguments; interpret them as newline-separated) - rust-lang#73382 (Only display other method receiver candidates if they actually apply) - rust-lang#73465 (Add specialization of `ToString for char`) - rust-lang#73489 (Refactor hir::Place) Failed merges: r? @ghost

* when newer rust is used, iotedge-* fails with: with 1.47.0 and 1.46.0 rust_2018_idioms, triggers unused_attributes warning because rust_2018_idioms is for crate level only since: rust-lang/rust#73300 which is then an error because warnings are denied as well: | ''+v8+v8' is not a recognized feature for this target' is not a recognized feature for this target (ignoring feature) | (ignoring feature) | '+v8' is not a recognized feature for this target' (ignoring feature) | +v8' is not a recognized feature for this target (ignoring feature) | error: deny(rust_2018_idioms) is ignored unless specified at crate level | --> edgelet-http/src/authentication.rs:3:9 | | | 3 | #![deny(rust_2018_idioms, warnings)] | | ^^^^^^^^^^^^^^^^ | | | note: the lint level is defined here | --> edgelet-http/src/authentication.rs:3:27 | | | 3 | #![deny(rust_2018_idioms, warnings)] | | ^^^^^^^^ | = note: `#[deny(unused_attributes)]` implied by `#[deny(warnings)]` | | error: deny(rust_2018_idioms) is ignored unless specified at crate level | --> edgelet-http/src/certificate_manager.rs:1:9 | | | 1 | #![deny(rust_2018_idioms, warnings)] | | ^^^^^^^^^^^^^^^^ | | | note: the lint level is defined here | --> edgelet-http/src/certificate_manager.rs:1:27 | | | 1 | #![deny(rust_2018_idioms, warnings)] | | ^^^^^^^^ | | error: deny(rust_2018_idioms) is ignored unless specified at crate level | --> edgelet-http/src/authentication.rs:3:9 | | | 3 | #![deny(rust_2018_idioms, warnings)] | | ^^^^^^^^^^^^^^^^ | | error: deny(rust_2018_idioms) is ignored unless specified at crate level | --> edgelet-http/src/certificate_manager.rs:1:9 | | | 1 | #![deny(rust_2018_idioms, warnings)] | | ^^^^^^^^^^^^^^^^ | | ''+v8+v8' is not a recognized feature for this target' is not a recognized feature for this target (ignoring feature) | (ignoring feature) | '+v8' is not a recognized feature for this target (ignoring feature) | '+v8' is not a recognized feature for this target (ignoring feature) with 1.43.0 it still fails due to use of deprecated description (since 1.42.0 with rust-lang/rust#66919) | error: use of deprecated item 'std::error::Error::description': use the Display impl or to_string() | --> iotedge/src/support_bundle.rs:248:54 | | | 248 | let err_message = inspect.err().unwrap().description().to_owned(); | | ^^^^^^^^^^^ | | | note: the lint level is defined here | --> iotedge/src/lib.rs:3:27 | | | 3 | #![deny(rust_2018_idioms, warnings)] | | ^^^^^^^^ | = note: `#[deny(deprecated)]` implied by `#[deny(warnings)]` | | error: use of deprecated item 'std::error::Error::description': use the Display impl or to_string() | --> iotedge/src/support_bundle.rs:302:54 | | | 302 | let err_message = inspect.err().unwrap().description().to_owned(); | | ^^^^^^^^^^^ | | error: use of deprecated item 'std::error::Error::description': use the Display impl or to_string() | --> iotedge/src/support_bundle.rs:375:54 | | | 375 | let err_message = inspect.err().unwrap().description().to_owned(); | | ^^^^^^^^^^^ | | error: use of deprecated item 'std::error::Error::description': use the Display impl or to_string() | --> iotedge/src/support_bundle.rs:457:54 | | | 457 | let err_message = inspect.err().unwrap().description().to_owned(); | | ^^^^^^^^^^^ you can either work around it by allowing warnings (deprecated and unused_attributes): find ${WORKDIR}/iotedge-${PV} -name "*.rs" -exec sed -i 's/idioms, warnings)/idioms)/g' {} \; or fix it properly in next iotedge release, or just select older rust version which doesn't trigger fatal error for this with: RUST_VERSION = "1.41.0" PREFERRED_VERSION_rust-native ?= "${RUST_VERSION}" PREFERRED_VERSION_rust-cross-${TARGET_ARCH} ?= "${RUST_VERSION}" PREFERRED_VERSION_rust-llvm-native ?= "${RUST_VERSION}" PREFERRED_VERSION_libstd-rs ?= "${RUST_VERSION}" PREFERRED_VERSION_cargo-native ?= "${RUST_VERSION}" Signed-off-by: Martin Jansa <martin.jansa@lge.com>

rust-highfive assigned davidtwco Jun 13, 2020

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 13, 2020

crlf0710 mentioned this pull request Jun 13, 2020

Errors reported in rustc_lint::levels::LintLevelsBuilder::push occurs multiple times #73301

Closed

rust-highfive assigned petrochenkov and unassigned davidtwco Jun 13, 2020

crlf0710 force-pushed the crate_level_only_check branch 2 times, most recently from 61cfc0c to 56f4ab7 Compare June 13, 2020 05:33

petrochenkov reviewed Jun 13, 2020

View reviewed changes

src/test/ui/lint/crate_level_only_lint.rs Outdated Show resolved Hide resolved

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 13, 2020

crlf0710 force-pushed the crate_level_only_check branch from 56f4ab7 to c9bba88 Compare June 15, 2020 18:45

crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 15, 2020

crlf0710 requested a review from petrochenkov June 15, 2020 18:56

This comment has been minimized.

Sign in to view

crlf0710 force-pushed the crate_level_only_check branch from c9bba88 to 12ef4f6 Compare June 16, 2020 01:48

This comment has been minimized.

Sign in to view

crlf0710 force-pushed the crate_level_only_check branch from 12ef4f6 to 644410d Compare June 16, 2020 15:47

crlf0710 force-pushed the crate_level_only_check branch from 6684a79 to 7f27320 Compare June 17, 2020 12:57

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 17, 2020

Implement crate level only lints checking.

f633dd3

crlf0710 force-pushed the crate_level_only_check branch from 7f27320 to f633dd3 Compare June 18, 2020 01:54

crlf0710 added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 18, 2020

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 Jun 18, 2020

This was referenced Jun 19, 2020

Rollup of 10 pull requests #73499

Closed

Rollup of 13 pull requests #73511

Merged

bors merged commit 058971c into rust-lang:master Jun 20, 2020

crlf0710 mentioned this pull request Jun 22, 2020

Tracking issue for RFC 2457, "Allow non-ASCII identifiers" #55467

Closed

23 tasks

ehuss mentioned this pull request Jul 17, 2020

#![allow(elided_lifetimes_in_paths)] doesn't work in submodules #74463

Closed

zackmdavis mentioned this pull request Jan 16, 2021

Can't allow elided_lifetimes_in_paths inside a module that denies rust_2018_idioms, other lints work #71957

Closed

cuviper added this to the 1.46 milestone May 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement crate-level-only lints checking. #73300

Implement crate-level-only lints checking. #73300

crlf0710 commented Jun 13, 2020 •

edited

Loading

rust-highfive commented Jun 13, 2020

rust-highfive commented Jun 13, 2020

crlf0710 commented Jun 13, 2020

ehuss commented Jun 13, 2020

crlf0710 commented Jun 13, 2020

crlf0710 commented Jun 13, 2020 •

edited

Loading

petrochenkov commented Jun 13, 2020 •

edited

Loading

petrochenkov commented Jun 13, 2020 •

edited

Loading

crlf0710 commented Jun 15, 2020 •

edited

Loading

crlf0710 commented Jun 15, 2020 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Jun 16, 2020

petrochenkov commented Jun 16, 2020 •

edited

Loading

petrochenkov commented Jun 17, 2020

crlf0710 commented Jun 18, 2020

petrochenkov commented Jun 18, 2020

bors commented Jun 18, 2020

Implement crate-level-only lints checking. #73300

Implement crate-level-only lints checking. #73300

Conversation

crlf0710 commented Jun 13, 2020 • edited Loading

rust-highfive commented Jun 13, 2020

rust-highfive commented Jun 13, 2020

crlf0710 commented Jun 13, 2020

ehuss commented Jun 13, 2020

crlf0710 commented Jun 13, 2020

crlf0710 commented Jun 13, 2020 • edited Loading

petrochenkov commented Jun 13, 2020 • edited Loading

petrochenkov commented Jun 13, 2020 • edited Loading

crlf0710 commented Jun 15, 2020 • edited Loading

crlf0710 commented Jun 15, 2020 • edited Loading

This comment has been minimized.

This comment has been minimized.

petrochenkov commented Jun 16, 2020

petrochenkov commented Jun 16, 2020 • edited Loading

petrochenkov commented Jun 17, 2020

crlf0710 commented Jun 18, 2020

petrochenkov commented Jun 18, 2020

bors commented Jun 18, 2020

crlf0710 commented Jun 13, 2020 •

edited

Loading

crlf0710 commented Jun 13, 2020 •

edited

Loading

petrochenkov commented Jun 13, 2020 •

edited

Loading

petrochenkov commented Jun 13, 2020 •

edited

Loading

crlf0710 commented Jun 15, 2020 •

edited

Loading

crlf0710 commented Jun 15, 2020 •

edited

Loading

petrochenkov commented Jun 16, 2020 •

edited

Loading