Skip to content

Lint Lints without LintPass #1207

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

Merged
merged 1 commit into from
Sep 1, 2016
Merged

Lint Lints without LintPass #1207

merged 1 commit into from
Sep 1, 2016

Conversation

kennytm
Copy link
Member

@kennytm kennytm commented Aug 31, 2016

Added a lint_without_lint_pass lint.

Four lints were missing from LintPass, making them unavailable unless the clippy lint group is explicitly enabled:

  • for_loop_over_result
  • for_loop_over_option
  • match_overlapping_arm
  • filter_next

@mcarton
Copy link
Member

mcarton commented Aug 31, 2016

Thanks!
Travis is not happy because you need to run ./util/update_lints.py.

}


pub static IGNORE_ME_PLEASE: u32 = 0;
Copy link
Member

Choose a reason for hiding this comment

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

Why? 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops, forgot to remove it 😛

@mcarton
Copy link
Member

mcarton commented Aug 31, 2016

  • You should move the lint to rust-clippy/clippy_lints/src/utils/internal_lints.rs.

//(DISABLED)~^ ERROR: the lint MISSING_LINT is not added to any LintPass</del>

// FIXME: Either the JSON output or compiletest-rs is wrong. The error above is
// not captured.
Copy link
Member

Choose a reason for hiding this comment

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

That's weird. The error is in the JSON:

{"message":"the lint MISSING_LINT is not added to any LintPass","code":null,"level":"error","spans":[{"file_name":"<rustc macros>","byte_start":6188,"byte_end":6293,"line_start":5,"line_end":6,"column_start":1,"column_end":30,"is_primary":true,"text":[{"text":"static $ name : & 'static :: rustc :: lint :: Lint = & lint_initializer ! (","highlight_start":1,"highlight_end":76},{"text":"$ name , $ level , $ desc ) ; ) ;","highlight_start":1,"highlight_end":30}],"label":null,"suggested_replacement":null,"expansion":{"span":{"file_name":"tests/compile-fail/lint_pass.rs","byte_start":234,"byte_end":286,"line_start":13,"line_end":13,"column_start":1,"column_end":53,"is_primary":false,"text":[{"text":"declare_lint! { MISSING_LINT, Warn, \"missing lint\" }","highlight_start":1,"highlight_end":53}],"label":null,"suggested_replacement":null,"expansion":null},"macro_decl_name":"declare_lint!","def_site_span":{"file_name":"<rustc macros>","byte_start":5954,"byte_end":6297,"line_start":1,"line_end":6,"column_start":1,"column_end":34,"is_primary":false,"text":[{"text":"( pub $ name : ident , $ level : ident , $ desc : expr ) => (","highlight_start":1,"highlight_end":62},{"text":"pub static $ name : & 'static :: rustc :: lint :: Lint = & lint_initializer !","highlight_start":1,"highlight_end":78},{"text":"( $ name , $ level , $ desc ) ; ) ; (","highlight_start":1,"highlight_end":38},{"text":"$ name : ident , $ level : ident , $ desc : expr ) => (","highlight_start":1,"highlight_end":56},{"text":"static $ name : & 'static :: rustc :: lint :: Lint = & lint_initializer ! (","highlight_start":1,"highlight_end":76},{"text":"$ name , $ level , $ desc ) ; ) ;","highlight_start":1,"highlight_end":34}],"label":null,"suggested_replacement":null,"expansion":null}}}],"children":[{"message":"lint level defined here","code":null,"level":"note","spans":[{"file_name":"tests/compile-fail/lint_pass.rs","byte_start":75,"byte_end":97,"line_start":5,"line_end":5,"column_start":9,"column_end":31,"is_primary":true,"text":[{"text":"#![deny(lint_without_lint_pass)]","highlight_start":9,"highlight_end":31}],"label":null,"suggested_replacement":null,"expansion":null}],"children":[],"rendered":null},{"message":"for further information visit https://github.com/Manishearth/rust-clippy/wiki#lint_without_lint_pass","code":null,"level":"help","spans":[],"children":[],"rendered":null}],"rendered":null}

but does not seem to be seen by compiletest-rs:

actual errors (from JSON output): [
    Error {
        line_num: 5,
        kind: Some(
            Note
        ),
        msg: "5:9: 5:31: lint level defined here"
    }
]

expected errors (from test file): [
    Error {
        line_num: 5,
        kind: Some(
            Note
        ),
        msg: ""
    },
    Error {
        line_num: 13,
        kind: Some(
            Error
        ),
        msg: "the lint MISSING_LINT is not added to any LintPass</del>"
    }
]

Copy link
Member

@mcarton mcarton Aug 31, 2016

Choose a reason for hiding this comment

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

Ah no… I see, the JSON has "file_name":"<rustc macros>" as the span. compiletest-rs thinks that's an error in a different file and just ignores it. See if you can get another span for that.

Copy link
Member

Choose a reason for hiding this comment

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

*/!\ SPOILER ALERT /!*

diff --git a/clippy_lints/src/lint_pass.rs b/clippy_lints/src/lint_pass.rs
index e041ee4..6840420 100644
--- a/clippy_lints/src/lint_pass.rs
+++ b/clippy_lints/src/lint_pass.rs
@@ -66,12 +66,13 @@ impl LateLintPass for Pass {
     }

     fn check_crate_post(&mut self, cx: &LateContext, _: &Crate) {
-        for (lint_name, lint_span) in &self.declared_lints {
+        for (lint_name, &lint_span) in &self.declared_lints {
+            let lint_span = cx.sess().codemap().source_callsite(lint_span);
             if !self.registered_lints.contains(lint_name) {
                 span_lint(cx,
                           LINT_WITHOUT_LINT_PASS,
-                          *lint_span,
-                          &format!("the lint {} is not added to any LintPass", lint_name));
+                          lint_span,
+                          &format!("the lint `{}` is not added to any `LintPass`", lint_name));
             }
         }
     }

Copy link
Contributor

Choose a reason for hiding this comment

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

Whoa! Didn't see that one coming!

@kennytm
Copy link
Member Author

kennytm commented Sep 1, 2016

@mcarton Thanks! Add changes amended.

Four lints were missing from LintPass, making them unavailable unless the
`clippy` lint group is explicitly enabled:

* `for_loop_over_result`
* `for_loop_over_option`
* `match_overlapping_arm`
* `filter_next`
@mcarton mcarton merged commit 5f09020 into rust-lang:master Sep 1, 2016
@mcarton
Copy link
Member

mcarton commented Sep 1, 2016

Thanks!

kennytm referenced this pull request Sep 1, 2016
mcarton added a commit that referenced this pull request Sep 2, 2016
yati-sagade pushed a commit to yati-sagade/rust-clippy that referenced this pull request Dec 31, 2016
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.

3 participants