OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Rollup merge of #52851 - flip1995:tool_lints, r=oli-obk

Browse Source 
Make the tool_lints actually usable

cc #44690

Necessary for rust-lang-nursery/rust-clippy#2955 and rust-lang-nursery/rust-clippy#2977

This PR makes it possible for lint tools (at the moment only for Clippy) to implement the `tool_lints`, like it was documented in #52018.

Because the `declare_lint` macro is pretty cluttered right now, there was not really a good way to add the `tool_name` as an additional argument of the macro. That's why I chose to introduce the new `declare_tool_lint` macro.

The switch from `&str` to `String` in the `lint_groups` `FxHashMap` is because I got weird error messages in the `check_lint_name` method. And the `by_name` field of the `LintStore` also uses `String`.

### What comes with this PR:

If this PR lands and Clippy switches to the `tool_lints`, the currently used methods
```rust
#[cfg_attr(feature = "cargo-clippy", allow(clippy_lint))]
#[allow(unknown_lints, clippy_lint)]
```
to `allow`/`warn`/`deny`/`forbid` Clippy lints, won't have any effects anymore, but also won't produce a warning. That is because the name of `clippy_lint` will then be `clippy::clippy_lint`. (Maybe we can add a clippy lint to search for `cfg_attr` appearances with the `cargo-clippy` feature?)

r? @oli-obk
This commit is contained in:
Pietro Albini 2018-08-01 10:12:51 +02:00 committed by GitHub
commit 5fb7c65f54
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 172 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
src/librustc/lint/context.rs
View File

@ -42,7 +42,7 @@ use util::nodemap::FxHashMap;
use std::default::Default as StdDefault;
use syntax::ast;
use syntax::edition;
use syntax_pos::{MultiSpan, Span};
use syntax_pos::{MultiSpan, Span, symbol::LocalInternedString};
use errors::DiagnosticBuilder;
use hir;
use hir::def_id::LOCAL_CRATE;
@ -133,6 +133,12 @@ pub enum CheckLintNameResult<'a> {
/// The lint is either renamed or removed. This is the warning
/// message, and an optional new name (`None` if removed).
Warning(String, Option<String>),
/// The lint is from a tool. If the Option is None, then either
/// the lint does not exist in the tool or the code was not
/// compiled with the tool and therefore the lint was never
/// added to the `LintStore`. Otherwise the `LintId` will be
/// returned as if it where a rustc lint.
Tool(Option<&'a [LintId]>),
}
impl LintStore {
@ -288,7 +294,7 @@ impl LintStore {
sess: &Session,
lint_name: &str,
level: Level) {
let db = match self.check_lint_name(lint_name) {
let db = match self.check_lint_name(lint_name, None) {
CheckLintNameResult::Ok(_) => None,
CheckLintNameResult::Warning(ref msg, _) => {
Some(sess.struct_warn(msg))
@ -296,6 +302,7 @@ impl LintStore {
CheckLintNameResult::NoLint => {
Some(struct_err!(sess, E0602, "unknown lint: `{}`", lint_name))
}
CheckLintNameResult::Tool(_) => unreachable!(),
};
if let Some(mut db) = db {
@ -319,26 +326,41 @@ impl LintStore {
/// it emits non-fatal warnings and there are *two* lint passes that
/// inspect attributes, this is only run from the late pass to avoid
/// printing duplicate warnings.
pub fn check_lint_name(&self, lint_name: &str) -> CheckLintNameResult {
match self.by_name.get(lint_name) {
Some(&Renamed(ref new_name, _)) => {
CheckLintNameResult::Warning(
format!("lint `{}` has been renamed to `{}`", lint_name, new_name),
Some(new_name.to_owned())
)
},
Some(&Removed(ref reason)) => {
CheckLintNameResult::Warning(
format!("lint `{}` has been removed: `{}`", lint_name, reason),
None
)
},
None => {
match self.lint_groups.get(lint_name) {
None => CheckLintNameResult::NoLint,
Some(ids) => CheckLintNameResult::Ok(&ids.0),
}
pub fn check_lint_name(
&self,
lint_name: &str,
tool_name: Option<LocalInternedString>,
) -> CheckLintNameResult {
let complete_name = if let Some(tool_name) = tool_name {
format!("{}::{}", tool_name, lint_name)
} else {
lint_name.to_string()
};
if let Some(_) = tool_name {
match self.by_name.get(&complete_name) {
None => match self.lint_groups.get(&*complete_name) {
None => return CheckLintNameResult::Tool(None),
Some(ids) => return CheckLintNameResult::Tool(Some(&ids.0)),
},
Some(&Id(ref id)) => return CheckLintNameResult::Tool(Some(slice::from_ref(id))),
// If the lint was registered as removed or renamed by the lint tool, we don't need
// to treat tool_lints and rustc lints different and can use the code below.
_ => {}
}
}
match self.by_name.get(&complete_name) {
Some(&Renamed(ref new_name, _)) => CheckLintNameResult::Warning(
format!("lint `{}` has been renamed to `{}`", lint_name, new_name),
Some(new_name.to_owned()),
),
Some(&Removed(ref reason)) => CheckLintNameResult::Warning(
format!("lint `{}` has been removed: `{}`", lint_name, reason),
None,
),
None => match self.lint_groups.get(&*complete_name) {
None => CheckLintNameResult::NoLint,
Some(ids) => CheckLintNameResult::Ok(&ids.0),
},
Some(&Id(ref id)) => CheckLintNameResult::Ok(slice::from_ref(id)),
}
}

37
src/librustc/lint/levels.rs
View File

@ -227,8 +227,10 @@ impl<'a> LintLevelsBuilder<'a> {
continue
}
};
if let Some(lint_tool) = word.is_scoped() {
if !self.sess.features_untracked().tool_lints {
let tool_name = if let Some(lint_tool) = word.is_scoped() {
let gate_feature = !self.sess.features_untracked().tool_lints;
let known_tool = attr::is_known_lint_tool(lint_tool);
if gate_feature {
feature_gate::emit_feature_err(&sess.parse_sess,
"tool_lints",
word.span,
@ -236,8 +238,7 @@ impl<'a> LintLevelsBuilder<'a> {
&format!("scoped lint `{}` is experimental",
word.ident));
}
if !attr::is_known_lint_tool(lint_tool) {
if !known_tool {
span_err!(
sess,
lint_tool.span,
@ -247,10 +248,16 @@ impl<'a> LintLevelsBuilder<'a> {
);
}
continue
}
if gate_feature || !known_tool {
continue
}
Some(lint_tool.as_str())
} else {
None
};
let name = word.name();
match store.check_lint_name(&name.as_str()) {
match store.check_lint_name(&name.as_str(), tool_name) {
CheckLintNameResult::Ok(ids) => {
let src = LintSource::Node(name, li.span);
for id in ids {
@ -258,6 +265,20 @@ impl<'a> LintLevelsBuilder<'a> {
}
}
CheckLintNameResult::Tool(result) => {
if let Some(ids) = result {
let complete_name = &format!("{}::{}", tool_name.unwrap(), name);
let src = LintSource::Node(Symbol::intern(complete_name), li.span);
for id in ids {
specs.insert(*id, (level, src));
}
}
// If Tool(None) is returned, then either the lint does not exist in the
// tool or the code was not compiled with the tool and therefore the lint
// was never added to the `LintStore`. To detect this is the responsibility
// of the lint tool.
}
_ if !self.warn_about_weird_lints => {}
CheckLintNameResult::Warning(msg, renamed) => {
@ -298,7 +319,7 @@ impl<'a> LintLevelsBuilder<'a> {
if name.as_str().chars().any(|c| c.is_uppercase()) {
let name_lower = name.as_str().to_lowercase().to_string();
if let CheckLintNameResult::NoLint =
store.check_lint_name(&name_lower) {
store.check_lint_name(&name_lower, tool_name) {
db.emit();
} else {
db.span_suggestion_with_applicability(

20
src/librustc/lint/mod.rs
View File

@ -139,6 +139,26 @@ macro_rules! declare_lint {
);
}
#[macro_export]
macro_rules! declare_tool_lint {
($vis: vis $tool: ident ::$NAME: ident, $Level: ident, $desc: expr) => (
declare_tool_lint!{$vis $tool::$NAME, $Level, $desc, false}
);
($vis: vis $tool: ident ::$NAME: ident, $Level: ident, $desc: expr,
report_in_external_macro: $rep: expr) => (
declare_tool_lint!{$vis $tool::$NAME, $Level, $desc, $rep}
);
($vis: vis $tool: ident ::$NAME: ident, $Level: ident, $desc: expr, $external: expr) => (
$vis static $NAME: &$crate::lint::Lint = &$crate::lint::Lint {
name: &concat!(stringify!($tool), "::", stringify!($NAME)),
default_level: $crate::lint::$Level,
desc: $desc,
edition_lint_opts: None,
report_in_external_macro: $external,
};
);
}
/// Declare a static `LintArray` and return it as an expression.
#[macro_export]
macro_rules! lint_array {

48
src/test/ui-fulldeps/auxiliary/lint_tool_test.rs Normal file
View File

@ -0,0 +1,48 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(plugin_registrar)]
#![feature(box_syntax, rustc_private)]
#![feature(macro_vis_matcher)]
#![feature(macro_at_most_once_rep)]
extern crate syntax;
// Load rustc as a plugin to get macros
#[macro_use]
extern crate rustc;
extern crate rustc_plugin;
use rustc::lint::{EarlyContext, LintContext, LintPass, EarlyLintPass,
LintArray};
use rustc_plugin::Registry;
use syntax::ast;
declare_tool_lint!(pub clippy::TEST_LINT, Warn, "Warn about stuff");
struct Pass;
impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(TEST_LINT)
}
}
impl EarlyLintPass for Pass {
fn check_item(&mut self, cx: &EarlyContext, it: &ast::Item) {
if it.ident.name == "lintme" {
cx.span_lint(TEST_LINT, it.span, "item is named 'lintme'");
}
}
}
#[plugin_registrar]
pub fn plugin_registrar(reg: &mut Registry) {
reg.register_early_lint_pass(box Pass);
}

24
src/test/ui-fulldeps/lint_tool_test.rs Normal file
View File

@ -0,0 +1,24 @@
// Copyright 2018 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
// run-pass
// aux-build:lint_tool_test.rs
// ignore-stage1
#![feature(plugin)]
#![feature(tool_lints)]
#![plugin(lint_tool_test)]
#![allow(dead_code)]
fn lintme() { } //~ WARNING item is named 'lintme'
#[allow(clippy::test_lint)]
pub fn main() {
fn lintme() { }
}

8
src/test/ui-fulldeps/lint_tool_test.stderr Normal file
View File

@ -0,0 +1,8 @@
warning: item is named 'lintme'
--> $DIR/lint_tool_test.rs:19:1
|
LL | fn lintme() { } //~ WARNING item is named 'lintme'
| ^^^^^^^^^^^^^^^
|
= note: #[warn(clippy::test_lint)] on by default