Merge pull request #288 from xFrednet/278-lint-path-names

API: Include the lint crate in the lint identifier
rust-marker · Oct 12, 2023 · 886086e · 886086e
2 parents 811e840 + 18eb84d
commit 886086e
Show file tree

Hide file tree

Showing 45 changed files with 100 additions and 76 deletions.
diff --git a/.github/workflows/gh-pages.yml b/.github/workflows/gh-pages.yml
@@ -10,7 +10,7 @@ on:
  # Match only on specific tags. We don't want this workflow to be invoked when
  # we put sliding `v{major}` and `v{major}.{minor}` tags on the same commit.
  tags: ['v[0-9]+.[0-9]+.[0-9]+*']
-  workflow_dispatch:
+ workflow_dispatch:
 
 env:
  CARGO_TERM_COLOR: always

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -27,11 +27,13 @@ The [v0.4.0 milestone] contains a list of planned changes.
 
 [v0.4.0 milestone]: https://github.com/rust-marker/marker/milestone/4
 [#278]: https://github.com/rust-marker/marker/pull/278
+[#288]: https://github.com/rust-marker/marker/pull/288
 
 ### Breaking Changes
 - [#278]: Moved the `span()` method from the inherent impl to `HasSpan` trait for `ClosureParam`, `FnParam`, `StructFieldPat`.
 - [#278]: The trait `TyData` no longer has own `span()` method, but instead requires `HasSpan` as a supertrait.
 - [#278]: All public methods that took `&Span` as a parameter now take `impl HasSpan`. This is a minor breaking change, as `&Span` implements `HasSpan`, but if you relied on type inference based on the function parameter type, then making a method generic may break your code.
+- [#288]: Lint identifiers use the lint crate name as a new infix, to prevent name collisions across lint crates.
 
 ## [0.3.0] - 2023-10-05
 

diff --git a/marker_api/src/lint.rs b/marker_api/src/lint.rs
@@ -30,15 +30,16 @@ use crate::common::{Level, MacroReport};
 #[repr(C)]
 #[derive(Debug, PartialEq, Eq, Hash)]
 pub struct Lint {
- /// A string identifier for the lint.
+ /// The string identifier of this lint.
  ///
- /// This identifies the lint in attributes and in command-line arguments.
- /// In those contexts it is always lowercase. This allows
- /// [`declare_lint`](crate::declare_lint) macro invocations to follow the
- /// convention of upper-case statics without repeating the name.
+ /// Identifiers use underscores, to connect multiple words, e.g., "unused_imports".
+ /// Lint names in console arguments use dashes instead of underscores. These are
+ /// automatically converted to underscores internally.
  ///
- /// The name is written with underscores, e.g., "unused_imports".
- /// On the command line, underscores become dashes.
+ /// Identifiers in Marker are split into three parts:
+ /// 1. The `marker` prefix, specifying that the lint comes from Marker
+ /// 2. The lint crate name, as an infix
+ /// 3. The name of the lint, as the postfix
  ///
  /// See <https://rustc-dev-guide.rust-lang.org/diagnostics.html#lint-naming>
  /// for naming guidelines.
@@ -118,7 +119,23 @@ macro_rules! declare_lint {
  ) => {
  $(#[doc = $doc])+
  pub static $NAME: &$crate::Lint = &$crate::Lint {
- name: concat!("marker::", stringify!($NAME)),
+ // The `CARGO_CRATE_NAME` environment value is set by Cargo during compilation.
+ // The name has already been normalized to use underscores, instead of dashes,
+ // which is ideal for Marker.
+ //
+ // The name is taken from the `Cargo.toml` file of the lint crate, even if the
+ // dependency has been renamed. This is fine for now, since Marker doesn't expose
+ // any interface for the users to rename the lint crates. We might need to add our
+ // own environment value later, if we want to support lint crate renaming.
+ //
+ // Example, how the package could be renamed in the `Cargo.toml` of Marker's dummy
+ // crate for lint crate fetching:
+ // ```toml
+ // [dependencies]
+ // ducks = { version = "0.3", package = "marker_lints" }
+ // ```
+ // The environment value would still have the value `marker_lints`
+ name: concat!("marker::", std::env!("CARGO_CRATE_NAME"), "::", stringify!($NAME)),
  default_level: $crate::common::Level::$LEVEL,
  explanation: concat!($($doc, '\n',)*),
  report_in_macro: $REPORT_IN_MACRO,

diff --git a/marker_lints/tests/ui/diag_msg_uppercase_start.stderr b/marker_lints/tests/ui/diag_msg_uppercase_start.stderr
@@ -4,7 +4,7 @@ warning: this message starts with an uppercase character
 21 | cx.emit_lint(DUMMY, expr, "X <-- starting with upper case");
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
- = note: `#[warn(marker::diag_msg_uppercase_start)]` on by default
+ = note: `#[warn(marker::marker_lints::diag_msg_uppercase_start)]` on by default
 
 warning: this message starts with an uppercase character
  --> $DIR/diag_msg_uppercase_start.rs:22:31

diff --git a/marker_lints/tests/ui/not_using_has_span_trait.rs b/marker_lints/tests/ui/not_using_has_span_trait.rs
@@ -2,7 +2,7 @@
  marker,
  feature(register_tool),
  register_tool(marker),
- warn(marker::not_using_has_span_trait)
+ warn(marker::marker_lints::not_using_has_span_trait)
 )]
 
 use marker_api::prelude::*;

diff --git a/marker_lints/tests/ui/not_using_has_span_trait.stderr b/marker_lints/tests/ui/not_using_has_span_trait.stderr
@@ -7,8 +7,8 @@ warning: use impl HasSpan instead of Span for more flexibility
 note: the lint level is defined here
  --> $DIR/not_using_has_span_trait.rs:5:10
  |
-5 | warn(marker::not_using_has_span_trait)
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+5 | warn(marker::marker_lints::not_using_has_span_trait)
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: use impl HasSpan instead of Span for more flexibility
  --> $DIR/not_using_has_span_trait.rs:11:19

diff --git a/marker_lints/tests/uitest.rs b/marker_lints/tests/uitest.rs
@@ -2,10 +2,15 @@ use marker_uitest::ui_test::*;
 use std::env;
 
 fn main() -> color_eyre::Result<()> {
- let mut config = marker_uitest::simple_ui_test_config!("tests/ui", "../target")?;
+ let mut config: Config = marker_uitest::simple_ui_test_config!("tests/ui", "../target")?;
 
  config.dependencies_crate_manifest_path = Some("./Cargo.toml".into());
 
+ // `marker_api::declare_lint` uses the `CARGO_CRATE_NAME` environment value.
+ // Setting it here once, makes sure that it'll be available, during the
+ // compilation of all ui test files.
+ std::env::set_var("CARGO_CRATE_NAME", "marker_lints_uitests");
+
  config.filter(r"\\/", "/");
  config.filter(r"\\\\", "/");
 

diff --git a/marker_uilints/tests/ui/cfg_attr_check.rs b/marker_uilints/tests/ui/cfg_attr_check.rs
@@ -2,12 +2,12 @@
 #![cfg_attr(marker, register_tool(marker))]
 
 mod allow_with_simple_attr {
- #[cfg_attr(marker, allow(marker::item_with_test_name))]
+ #[cfg_attr(marker, allow(marker::marker_uilints::item_with_test_name))]
  fn find_me_fn() {}
 }
 
 mod allow_with_crate_check_attr {
- #[cfg_attr(marker = "marker_uilints", allow(marker::item_with_test_name))]
+ #[cfg_attr(marker = "marker_uilints", allow(marker::marker_uilints::item_with_test_name))]
  fn find_me_fn() {}
 }
 
@@ -17,6 +17,6 @@ mod lint_with_unloaded_crate_attr {
 }
 
 mod unknown_lint_allow {
- #[cfg_attr(marker, allow(marker::some_unknown_lint_that_doesnt_exist))]
+ #[cfg_attr(marker, allow(marker::hey::some_unknown_lint_that_doesnt_exist))]
  fn foo() {}
 }
diff --git a/marker_uilints/tests/ui/cfg_attr_check.stderr b/marker_uilints/tests/ui/cfg_attr_check.stderr
@@ -1,8 +1,8 @@
-warning: unknown lint: `marker::some_unknown_lint_that_doesnt_exist`
+warning: unknown lint: `marker::hey::some_unknown_lint_that_doesnt_exist`
  --> $DIR/cfg_attr_check.rs:20:30
  |
-20 | #[cfg_attr(marker, allow(marker::some_unknown_lint_that_doesnt_exist))]
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+20 | #[cfg_attr(marker, allow(marker::hey::some_unknown_lint_that_doesnt_exist))]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: `#[warn(unknown_lints)]` on by default
 
@@ -12,7 +12,7 @@ warning: found a `fn` item with a test name
 16 | fn find_me_fn() {}
  | ^^^^^^^^^^^^^^^^^^
  |
- = note: `#[warn(marker::item_with_test_name)]` on by default
+ = note: `#[warn(marker::marker_uilints::item_with_test_name)]` on by default
 
 warning: 2 warnings emitted
 
diff --git a/marker_uilints/tests/ui/context/test_ast_map.stderr b/marker_uilints/tests/ui/context/test_ast_map.stderr
@@ -5,7 +5,7 @@ warning: testing `AstMap::variant`
  | ^^^^^^^^^^^^^^^^
  |
  = note: `AstMap::variant()` --> None
- = note: `#[warn(marker::test_ast_map)]` on by default
+ = note: `#[warn(marker::marker_uilints::test_ast_map)]` on by default
 
 warning: testing `AstMap::variant`
  --> $DIR/test_ast_map.rs:12:26

diff --git a/marker_uilints/tests/ui/expr/print_async_block.stderr b/marker_uilints/tests/ui/expr/print_async_block.stderr
@@ -59,7 +59,7 @@ warning: print test
  capture_kind: Move,
  },
  )
- = note: `#[warn(marker::test_lint)]` on by default
+ = note: `#[warn(marker::marker_uilints::test_lint)]` on by default
 
 warning: 1 warning emitted
 
diff --git a/marker_uilints/tests/ui/expr/print_await_expr.stderr b/marker_uilints/tests/ui/expr/print_await_expr.stderr
@@ -52,7 +52,7 @@ warning: print test
  ),
  },
  )
- = note: `#[warn(marker::test_lint)]` on by default
+ = note: `#[warn(marker::marker_uilints::test_lint)]` on by default
 
 warning: print test
  --> $DIR/print_await_expr.rs:9:5

diff --git a/marker_uilints/tests/ui/expr/print_closure_expr.stderr b/marker_uilints/tests/ui/expr/print_closure_expr.stderr
@@ -19,7 +19,7 @@ warning: print test
  body_id: BodyId(..),
  },
  )
- = note: `#[warn(marker::test_lint)]` on by default
+ = note: `#[warn(marker::marker_uilints::test_lint)]` on by default
 
 warning: print test
  --> $DIR/print_closure_expr.rs:7:5

diff --git a/marker_uilints/tests/ui/expr/sugar/README.md b/marker_uilints/tests/ui/expr/sugar/README.md
@@ -1 +1 @@
-This folder contains tests, to check that expressions are resugared correctly and their spans also match up.
+This folder contains tests, to check that expressions are resugared correctly and their spans also match up.
diff --git a/marker_uilints/tests/ui/expr/sugar/for_loop.rs b/marker_uilints/tests/ui/expr/sugar/for_loop.rs
@@ -5,7 +5,7 @@ fn main() {
  let range = 1..10;
  let mut total = 0;
 
- #[warn(marker::print_every_expr)]
+ #[warn(marker::marker_uilints::print_every_expr)]
  for i in range {
  total += i;
  }

diff --git a/marker_uilints/tests/ui/expr/sugar/for_loop.stderr b/marker_uilints/tests/ui/expr/sugar/for_loop.stderr
@@ -16,8 +16,8 @@ warning: expr
 note: the lint level is defined here
  --> $DIR/for_loop.rs:8:12
  |
-8 | #[warn(marker::print_every_expr)]
- | ^^^^^^^^^^^^^^^^^^^^^^^^
+8 | #[warn(marker::marker_uilints::print_every_expr)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: expr
  --> $DIR/for_loop.rs:9:14

diff --git a/marker_uilints/tests/ui/expr/sugar/ranges.rs b/marker_uilints/tests/ui/expr/sugar/ranges.rs
@@ -2,12 +2,12 @@
 #![register_tool(marker)]
 
 fn main() {
- #[warn(marker::print_every_expr)]
+ #[warn(marker::marker_uilints::print_every_expr)]
  let _ = 1..2;
- #[warn(marker::print_every_expr)]
+ #[warn(marker::marker_uilints::print_every_expr)]
  let _ = ..2;
- #[warn(marker::print_every_expr)]
+ #[warn(marker::marker_uilints::print_every_expr)]
  let _ = 1..;
- #[warn(marker::print_every_expr)]
+ #[warn(marker::marker_uilints::print_every_expr)]
  let _ = ..;
 }
diff --git a/marker_uilints/tests/ui/expr/sugar/ranges.stderr b/marker_uilints/tests/ui/expr/sugar/ranges.stderr
@@ -14,8 +14,8 @@ warning: expr
 note: the lint level is defined here
  --> $DIR/ranges.rs:5:12
  |
-5 | #[warn(marker::print_every_expr)]
- | ^^^^^^^^^^^^^^^^^^^^^^^^
+5 | #[warn(marker::marker_uilints::print_every_expr)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: expr
  --> $DIR/ranges.rs:6:13
@@ -61,8 +61,8 @@ warning: expr
 note: the lint level is defined here
  --> $DIR/ranges.rs:7:12
  |
-7 | #[warn(marker::print_every_expr)]
- | ^^^^^^^^^^^^^^^^^^^^^^^^
+7 | #[warn(marker::marker_uilints::print_every_expr)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: expr
  --> $DIR/ranges.rs:8:15
@@ -94,8 +94,8 @@ warning: expr
 note: the lint level is defined here
  --> $DIR/ranges.rs:9:12
  |
-9 | #[warn(marker::print_every_expr)]
- | ^^^^^^^^^^^^^^^^^^^^^^^^
+9 | #[warn(marker::marker_uilints::print_every_expr)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: expr
  --> $DIR/ranges.rs:10:13
@@ -127,8 +127,8 @@ warning: expr
 note: the lint level is defined here
  --> $DIR/ranges.rs:11:12
  |
-11 | #[warn(marker::print_every_expr)]
- | ^^^^^^^^^^^^^^^^^^^^^^^^
+11 | #[warn(marker::marker_uilints::print_every_expr)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: 8 warnings emitted
 
diff --git a/marker_uilints/tests/ui/expr/sugar/try_expr.rs b/marker_uilints/tests/ui/expr/sugar/try_expr.rs
@@ -2,14 +2,14 @@
 #![register_tool(marker)]
 
 fn try_something() -> Option<u32> {
- #[warn(marker::print_every_expr)]
+ #[warn(marker::marker_uilints::print_every_expr)]
  Some(21)?;
  None
 }
 
 fn kanske_option() -> Result<i32, u32> {
  let x: Result<i32, u32> = Err(1);
- #[warn(marker::print_every_expr)]
+ #[warn(marker::marker_uilints::print_every_expr)]
  x?;
  Ok(42)
 }

diff --git a/marker_uilints/tests/ui/expr/sugar/try_expr.stderr b/marker_uilints/tests/ui/expr/sugar/try_expr.stderr
@@ -14,8 +14,8 @@ warning: expr
 note: the lint level is defined here
  --> $DIR/try_expr.rs:5:12
  |
-5 | #[warn(marker::print_every_expr)]
- | ^^^^^^^^^^^^^^^^^^^^^^^^
+5 | #[warn(marker::marker_uilints::print_every_expr)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: expr
  --> $DIR/try_expr.rs:6:5
@@ -61,8 +61,8 @@ warning: expr
 note: the lint level is defined here
  --> $DIR/try_expr.rs:12:12
  |
-12 | #[warn(marker::print_every_expr)]
- | ^^^^^^^^^^^^^^^^^^^^^^^^
+12 | #[warn(marker::marker_uilints::print_every_expr)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: expr
  --> $DIR/try_expr.rs:13:5

diff --git a/marker_uilints/tests/ui/expr/sugar/while_loop.rs b/marker_uilints/tests/ui/expr/sugar/while_loop.rs
@@ -4,7 +4,7 @@
 fn main() {
  let mut something = Some(12);
 
- #[warn(marker::print_every_expr)]
+ #[warn(marker::marker_uilints::print_every_expr)]
  while let Some(_) = something {
  something = None;
  }

diff --git a/marker_uilints/tests/ui/expr/sugar/while_loop.stderr b/marker_uilints/tests/ui/expr/sugar/while_loop.stderr
@@ -16,8 +16,8 @@ warning: expr
 note: the lint level is defined here
  --> $DIR/while_loop.rs:7:12
  |
-7 | #[warn(marker::print_every_expr)]
- | ^^^^^^^^^^^^^^^^^^^^^^^^
+7 | #[warn(marker::marker_uilints::print_every_expr)]
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 warning: expr
  --> $DIR/while_loop.rs:8:11

diff --git a/marker_uilints/tests/ui/find_item.stderr b/marker_uilints/tests/ui/find_item.stderr
@@ -16,7 +16,7 @@ help: a spanned help
  |
 8 | static FIND_ITEM: u32 = 4;
  | ^^^^^^^^^^^^^^^^^^^^^^^^^^
- = note: `#[warn(marker::test_lint)]` on by default
+ = note: `#[warn(marker::marker_uilints::test_lint)]` on by default
 
 warning: 1 warning emitted
 
diff --git a/marker_uilints/tests/ui/foo_items.stderr b/marker_uilints/tests/ui/foo_items.stderr
@@ -10,7 +10,7 @@ warning: found a `module` item with a test name
 19 | | }
  | |_^
  |
- = note: `#[warn(marker::item_with_test_name)]` on by default
+ = note: `#[warn(marker::marker_uilints::item_with_test_name)]` on by default
 
 warning: found a `const` item with a test name
  --> $DIR/foo_items.rs:2:5

diff --git a/marker_uilints/tests/ui/item/print_async_fn.stderr b/marker_uilints/tests/ui/item/print_async_fn.stderr
@@ -72,7 +72,7 @@ warning: printing item with body
  },
  ),
  }
- = note: `#[warn(marker::test_lint)]` on by default
+ = note: `#[warn(marker::marker_uilints::test_lint)]` on by default
 
 warning: printing item with body
  --> $DIR/print_async_fn.rs:9:10

diff --git a/marker_uilints/tests/ui/item/print_fn_item.stderr b/marker_uilints/tests/ui/item/print_fn_item.stderr
@@ -32,7 +32,7 @@ warning: printing item
  ),
  },
  )
- = note: `#[warn(marker::test_lint)]` on by default
+ = note: `#[warn(marker::marker_uilints::test_lint)]` on by default
 
 warning: printing item
  --> $DIR/print_fn_item.rs:3:21

diff --git a/marker_uilints/tests/ui/item_id_resolution.stderr b/marker_uilints/tests/ui/item_id_resolution.stderr
@@ -48,7 +48,7 @@ warning: check type resolution
  = note: Is this a "std::string::String" -> false
  = note: Is this a "std::option::Option" -> false
  = note: Is this a "crate::TestType" -> false
- = note: `#[warn(marker::test_lint)]` on by default
+ = note: `#[warn(marker::marker_uilints::test_lint)]` on by default
 
 warning: check type resolution
  --> $DIR/item_id_resolution.rs:6:5

diff --git a/marker_uilints/tests/ui/lint_level_attributes.rs b/marker_uilints/tests/ui/lint_level_attributes.rs
@@ -3,8 +3,8 @@
 
 const FIND_ME_DEFAULT: i32 = 0;
 
-#[allow(marker::item_with_test_name)]
+#[allow(marker::marker_uilints::item_with_test_name)]
 const FIND_ME_ALLOW: i32 = 0;
 
-#[deny(marker::item_with_test_name)]
+#[deny(marker::marker_uilints::item_with_test_name)]
 const FIND_ME_DENY: i32 = 0;