Merge #214

214: 201/pass cfg flags r=xFrednet a=NathanFrasier

This is a little rough. I feel like there has to be a better way to get rid of some of the calls to `.push()` and `.extend()` but I wasn't thinking of them.

This still needs documentation as well.

Addresses #201 

Co-authored-by: Nathan Frasier <nathanfrasier.nf@gmail.com>

cargo-marker/src/backend.rs

@@ -111,7 +111,12 @@ pub fn run_check(config: &Config, info: CheckInfo, additional_cargo_args: &[Stri
 pub fn to_marker_lint_crates_env(lints: &[LintCrate]) -> OsString {
     let lint_paths: Vec<_> = lints
         .iter()
-        .map(|krate| OsString::from(krate.file.as_os_str()))
+        .map(|krate| {
+            let mut env_str = OsString::from(&krate.name);
+            env_str.push(":");
+            env_str.push(krate.file.as_os_str());
+            env_str
+        })
         .collect();
     lint_paths.join(OsStr::new(";"))
 }

cargo-marker/src/backend/lints.rs

@@ -23,6 +23,8 @@ pub struct LintCrateSource {
 /// The information of a compiled lint crate.
 #[derive(Debug)]
 pub struct LintCrate {
+    /// The name of the crate
+    pub name: String,
     /// The absolute path of the compiled crate, as a dynamic library.
     pub file: PathBuf,
 }

cargo-marker/src/backend/lints/build.rs

@@ -1,4 +1,4 @@
-use std::{ffi::OsStr, path::Path};
+use std::{collections::HashSet, ffi::OsStr, path::Path};
 
 use crate::{backend::Config, ExitStatus};
 
@@ -26,40 +26,52 @@ const ARTIFACT_ENDINGS: &[&str] = &[
 pub fn build_lints(sources: &[LintCrateSource], config: &Config) -> Result<Vec<LintCrate>, ExitStatus> {
     // By default Cargo doesn't provide the path of the compiled lint crate.
     // As a work around, we use the `--out-dir` option to make cargo copy all
-    // created binaries into one folder. We then scan that folder and collect
-    // all dynamic libraries, assuming that they're lint crates.
+    // created binaries into one folder. We then scan that folder as we build our crates
+    // and collect all dynamic libraries that show up, we link the name of the crate we just built
+    // with the file(s) we found after we built it, assuming that they are related.
+    //
+    // TODO Look into how to optimize this a bit better:
+    // Another option that would be potentially more performant is if we built each
+    // crate in it's own target dir. this would eliminate the need for HashSet<_> below, without
+    // changing too much else about the implementation.
+    //
+    // This would be so much simpler if we could get an output name from Cargo
 
     // Clear previously build lints
     let lints_dir = config.lint_crate_dir();
     clear_lints_dir(&lints_dir)?;
 
-    // Build lint crates
+    // Build lint crates and find the output of those builds
+    let mut found_paths = HashSet::new();
+    let ending = OsStr::new(DYNAMIC_LIB_FILE_ENDING);
+    let mut lints = Vec::with_capacity(sources.len());
+
     for lint_src in sources {
         build_lint(lint_src, config)?;
-    }
-
-    // Find lint lint crates
-    match std::fs::read_dir(&lints_dir) {
-        Ok(dir) => {
-            let ending = OsStr::new(DYNAMIC_LIB_FILE_ENDING);
-            let mut lints = vec![];
-            for file in dir {
-                let file = file.unwrap().path();
-                if file.extension() == Some(ending) {
-                    lints.push(LintCrate { file });
+        match std::fs::read_dir(&lints_dir) {
+            Ok(dir) => {
+                for file in dir {
+                    let file = file.unwrap().path();
+                    if file.extension() == Some(ending) && !found_paths.contains(&file) {
+                        found_paths.insert(file.clone());
+                        lints.push(LintCrate {
+                            file,
+                            name: lint_src.name.clone(),
+                        });
+                    }
                 }
-            }
-            Ok(lints)
-        },
-        Err(err) => {
-            // This shouldn't really be a point of failure. In this case, I'm
-            // more interested in the HOW?
-            panic!(
-                "unable to read lints dir after lint compilation: {} ({err:#?})",
-                lints_dir.display()
-            );
-        },
+            },
+            Err(err) => {
+                // This shouldn't really be a point of failure. In this case, I'm
+                // more interested in the HOW?
+                panic!(
+                    "unable to read lints dir after lint compilation: {} ({err:#?})",
+                    lints_dir.display()
+                );
+            },
+        }
     }
+    Ok(lints)
 }
 
 /// This function clears the `marker/lints` directory holding all compiled lints. This

docs/book/src/SUMMARY.md

@@ -3,6 +3,7 @@
 [Introduction](./README.md)
 - [Usage](./usage.md)
   - [Installation](./usage/installation.md)
+  - [Setting Lint Levels](./usage/setting-lint-levels.md)
   - [Lint Crate Declaration](./usage/lint-crate-declaration.md)
   - [Lint Crate Security](./usage/lint-crate-security.md)
 - [Lint Development](./lint-dev.md)

docs/book/src/usage/setting-lint-levels.md

@@ -0,0 +1,56 @@
+# Setting Lint Levels
+Once your crate is configured to run some lints, it quickly becomes important to control the lint levels within your
+code. Marker provides the ability to use normal lint control attributes like `#[allow(...)]` `#[deny(...)]` and others
+to control the behavior of marker lints.
+
+## On nightly
+Marker uses the `marker::` tool prefix for lints. In order for rustc to recognize this prefix, it requires marker to be
+registered via the [`#[register_tool()]` feature](https://github.com/rust-lang/rust/issues/66079).
+
+If your crate is compiled using nightly, then controlling lints is as simple as placing `#![feature(register_tool)]`
+and `#![register_tool(marker)]` at the top of your crate `lib.rs` or `mod.rs` file. You can then use all of the normal
+lint attributes you might usually use, on marker provided lints, like `#[allow(marker::my_lint)]`.
+
+
+#### Nightly Required Attributes in `lib.rs` / `main.rs`
+```rust
+#![feature(register_tool)]
+#![register_tool(marker)]
+```
+
+#### Nightly Lint Attribute
+```
+// Marker lints can be controlled like this
+#[allow(marker::my_lint)]
+fn foo() {}
+```
+
+## On Stable
+If your project isn't on nightly, you can still use the `register_tool` attribute, you'll just need to add some extra
+guards around it. Marker provides config arguments to rust during the lint passes, so that your linted code can
+conditionally apply attributes only during the marker run.
+
+Specifically marker passes `--cfg=marker` and a `--cfg=marker="my_crate"` flag for each lint crate. This means that you
+can use `#[cfg_attr(marker, foo)]` to conditionally apply the `foo` attribute only during lint runs.
+
+This conditional compilation can be used to leverage the fact that marker uses nightly for linting, without requiring
+the project to use a nightly toolchain. To do this, add `#![cfg_attr(marker, feature(register_tool))]` and
+`#![cfg_attr(marker, register_tool(marker))]` attributes to the top of your crate file, to register marker as a tool.
+Then you can apply lint level attributes like `#[cfg_attr(marker, allow(marker::foo))]` to control your marker lints.
+
+Additionally, you can check if a specific lint crate is in the set of loaded lint crates. This is useful, when you
+only want to enable some attributes if specific lints are loaded. For this you can use a `marker = "<lint-crate-name>"`
+check, like this: `#[cfg_attr(marker = "my_crate", allow(marker::foo))]`.
+
+#### Stable Required Attributes in `lib.rs` / `main.rs`
+```rust
+#![cfg_attr(marker, feature(register_tool))]
+#![cfg_attr(marker, register_tool(marker))]
+```
+
+#### Stable Conditional Lint Attribute
+```
+// Marker lints can be controlled like this
+#[cfg_attr(marker, allow(marker::my_lint))]
+fn foo() {}
+```

marker_adapter/README.md

@@ -26,7 +26,7 @@ Marker's API requires some callbacks from the lint crates into the driver. The a
 
 An adapter instance can be crated from the environment. For this, the following environment values are read:
 
-* `MARKER_LINT_CRATES`: A semicolon separated list of (absolute) paths to lint crates compiled as dynamic libraries.
+* `MARKER_LINT_CRATES`: A semicolon separated list of crate name and absolute path pairs. Each pair is internally separated by a colon.
 
 ## Contributing
 

marker_adapter/src/loader.rs

@@ -9,6 +9,8 @@ use super::{AdapterError, LINT_CRATES_ENV};
 /// A struct describing a lint crate that can be loaded
 #[derive(Debug, Clone)]
 pub struct LintCrateInfo {
+    /// The name of the lint crate
+    pub name: String,
     /// The absolute path of the compiled dynamic library, which can be loaded as a lint crate.
     pub path: PathBuf,
 }
@@ -25,9 +27,17 @@ impl LintCrateInfo {
     pub fn list_from_env() -> Result<Vec<LintCrateInfo>, AdapterError> {
         let env_str = std::env::var_os(LINT_CRATES_ENV).ok_or(AdapterError::LintCratesEnvUnset)?;
 
-        Ok(std::env::split_paths(&env_str)
-            .map(|path| LintCrateInfo { path })
-            .collect())
+        let mut lint_crates = vec![];
+        for item in env_str.to_str().ok_or(AdapterError::LintCratesEnvMalformed)?.split(';') {
+            let mut item_parts = item.splitn(2, ':');
+            let name = item_parts.next().ok_or(AdapterError::LintCratesEnvMalformed)?;
+            let path = item_parts.next().ok_or(AdapterError::LintCratesEnvMalformed)?;
+            lint_crates.push(LintCrateInfo {
+                name: name.to_string(),
+                path: PathBuf::from(path),
+            });
+        }
+        Ok(lint_crates)
     }
 }
 

marker_rustc_driver/src/main.rs

@@ -242,7 +242,7 @@ fn main() {
         // in preparation.
         if !has_sys_root_arg {
             let sys_root = find_sys_root(sys_root_arg);
-            orig_args.extend(vec!["--sysroot".into(), sys_root]);
+            orig_args.extend(["--sysroot".into(), sys_root]);
         };
 
         // make "marker_rustc_driver --rustc" work like a subcommand that passes
@@ -302,6 +302,17 @@ fn main() {
                 Err(marker_adapter::AdapterError::LintCratesEnvUnset) => vec![],
                 Err(err) => panic!("Error while determining the lint crates to load: {err:#?}"),
             };
+
+            // We need to provide a marker cfg flag to allow conditional compilation,
+            // we add a simple `marker` config for the common use case, but also provide
+            // `marker=crate_name` for more complex uses
+            orig_args.push("--cfg=marker".into());
+            orig_args.extend(
+                lint_crates
+                    .iter()
+                    .map(|krate| format!(r#"--cfg=marker="{}""#, krate.name)),
+            );
+
             let mut callback = MarkerCallback { env_vars, lint_crates };
             rustc_driver::RunCompiler::new(&orig_args, &mut callback).run()
         } else {

marker_uilints/tests/ui/cfg_attr_check.rs

@@ -0,0 +1,22 @@
+#![cfg_attr(marker, feature(register_tool))]
+#![cfg_attr(marker, register_tool(marker))]
+
+mod allow_with_simple_attr {
+    #[cfg_attr(marker, allow(marker::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))]
+    fn find_me_fn() {}
+}
+
+mod lint_with_unloaded_crate_attr {
+    #[cfg_attr(marker = "some_unknown_crate_that_isnt_loaded", allow(marker::item_with_test_name))]
+    fn find_me_fn() {}
+}
+
+mod unknown_lint_allow {
+    #[cfg_attr(marker, allow(marker::some_unknown_lint_that_doesnt_exist))]
+    fn foo() {}
+}

marker_uilints/tests/ui/cfg_attr_check.stderr

@@ -0,0 +1,18 @@
+warning: unknown lint: `marker::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))]
+   |                              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(unknown_lints)]` on by default
+
+warning: found a `fn` item with a test name
+  --> $DIR/cfg_attr_check.rs:16:5
+   |
+16 |     fn find_me_fn() {}
+   |     ^^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[warn(marker::item_with_test_name)]` on by default
+
+warning: 2 warnings emitted
+