Skip to content

Commit

Permalink
Add lint groups; define built-in lint groups bad_style and unused
Browse files Browse the repository at this point in the history
This adds support for lint groups to the compiler. Lint groups are a way of
grouping a number of lints together under one name. For example, this also
defines a default lint for naming conventions, named `bad_style`. Writing
`#[allow(bad_style)]` is equivalent to writing
`#[allow(non_camel_case_types, non_snake_case, non_uppercase_statics)]`. These
lint groups can also be defined as a compiler plugin using the new
`Registry::register_lint_group` method.

This also adds two built-in lint groups, `bad_style` and `unused`. The contents
of these groups can be seen by running `rustc -W help`.
  • Loading branch information
ftxqxd committed Aug 29, 2014
1 parent de7abd8 commit ed2aad8
Show file tree
Hide file tree
Showing 16 changed files with 318 additions and 50 deletions.
1 change: 0 additions & 1 deletion src/libcollections/hash/mod.rs
Expand Up @@ -157,7 +157,6 @@ macro_rules! impl_hash_tuple(

( $($name:ident)+) => (
impl<S: Writer, $($name: Hash<S>),*> Hash<S> for ($($name,)*) {
#[allow(uppercase_variables)]
#[inline]
#[allow(non_snake_case)]
fn hash(&self, state: &mut S) {
Expand Down
2 changes: 1 addition & 1 deletion src/libcore/ops.rs
Expand Up @@ -775,7 +775,7 @@ macro_rules! def_fn_mut(
FnMut<($($args,)*),Result>
for extern "Rust" fn($($args: $args,)*) -> Result {
#[rust_call_abi_hack]
#[allow(uppercase_variables)]
#[allow(non_snake_case)]
fn call_mut(&mut self, args: ($($args,)*)) -> Result {
let ($($args,)*) = args;
(*self)($($args,)*)
Expand Down
6 changes: 5 additions & 1 deletion src/librustc/driver/driver.rs
Expand Up @@ -242,13 +242,17 @@ pub fn phase_2_configure_and_expand(sess: &Session,
}
});

let Registry { syntax_exts, lint_passes, .. } = registry;
let Registry { syntax_exts, lint_passes, lint_groups, .. } = registry;

{
let mut ls = sess.lint_store.borrow_mut();
for pass in lint_passes.move_iter() {
ls.register_pass(Some(sess), true, pass);
}

for (name, to) in lint_groups.move_iter() {
ls.register_group(Some(sess), true, name, to);
}
}

// Lint plugins are registered; now we can process command line flags.
Expand Down
68 changes: 56 additions & 12 deletions src/librustc/driver/mod.rs
Expand Up @@ -180,14 +180,26 @@ Available lint options:
lints
}

fn sort_lint_groups(lints: Vec<(&'static str, Vec<lint::LintId>, bool)>)
-> Vec<(&'static str, Vec<lint::LintId>)> {
let mut lints: Vec<_> = lints.move_iter().map(|(x, y, _)| (x, y)).collect();
lints.sort_by(|&(x, _): &(&'static str, Vec<lint::LintId>),
&(y, _): &(&'static str, Vec<lint::LintId>)| {
x.cmp(&y)
});
lints
}

let (plugin, builtin) = lint_store.get_lints().partitioned(|&(_, p)| p);
let plugin = sort_lints(plugin);
let builtin = sort_lints(builtin);

// FIXME (#7043): We should use the width in character cells rather than
// the number of codepoints.
let (plugin_groups, builtin_groups) = lint_store.get_lint_groups().partitioned(|&(_, _, p)| p);
let plugin_groups = sort_lint_groups(plugin_groups);
let builtin_groups = sort_lint_groups(builtin_groups);

let max_name_len = plugin.iter().chain(builtin.iter())
.map(|&s| s.name.char_len())
.map(|&s| s.name.width(true))
.max().unwrap_or(0);
let padded = |x: &str| {
" ".repeat(max_name_len - x.char_len()).append(x)
Expand All @@ -208,16 +220,48 @@ Available lint options:

print_lints(builtin);

match (loaded_plugins, plugin.len()) {
(false, 0) => {
println!("Compiler plugins can provide additional lints. To see a listing of these, \
re-run `rustc -W help` with a crate filename.");


let max_name_len = plugin_groups.iter().chain(builtin_groups.iter())
.map(|&(s, _)| s.width(true))
.max().unwrap_or(0);
let padded = |x: &str| {
" ".repeat(max_name_len - x.char_len()).append(x)
};

println!("Lint groups provided by rustc:\n");
println!(" {} {}", padded("name"), "sub-lints");
println!(" {} {}", padded("----"), "---------");

let print_lint_groups = |lints: Vec<(&'static str, Vec<lint::LintId>)>| {
for (name, to) in lints.move_iter() {
let name = name.chars().map(|x| x.to_lowercase())
.collect::<String>().replace("_", "-");
let desc = to.move_iter().map(|x| x.as_str()).collect::<Vec<String>>().connect(", ");
println!(" {} {}",
padded(name.as_slice()), desc);
}
(false, _) => fail!("didn't load lint plugins but got them anyway!"),
(true, 0) => println!("This crate does not load any lint plugins."),
(true, _) => {
println!("Lint checks provided by plugins loaded by this crate:\n");
print_lints(plugin);
println!("\n");
};

print_lint_groups(builtin_groups);

match (loaded_plugins, plugin.len(), plugin_groups.len()) {
(false, 0, _) | (false, _, 0) => {
println!("Compiler plugins can provide additional lints and lint groups. To see a \
listing of these, re-run `rustc -W help` with a crate filename.");
}
(false, _, _) => fail!("didn't load lint plugins but got them anyway!"),
(true, 0, 0) => println!("This crate does not load any lint plugins or lint groups."),
(true, l, g) => {
if l > 0 {
println!("Lint checks provided by plugins loaded by this crate:\n");
print_lints(plugin);
}
if g > 0 {
println!("Lint groups provided by plugins loaded by this crate:\n");
print_lint_groups(plugin_groups);
}
}
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/librustc/lint/builtin.rs
Expand Up @@ -736,7 +736,7 @@ impl LintPass for UnusedResult {
}
}

declare_lint!(NON_CAMEL_CASE_TYPES, Warn,
declare_lint!(pub NON_CAMEL_CASE_TYPES, Warn,
"types, variants, traits and type parameters should have camel case names")

pub struct NonCamelCaseTypes;
Expand Down Expand Up @@ -844,7 +844,7 @@ fn method_context(cx: &Context, m: &ast::Method) -> MethodContext {
}
}

declare_lint!(NON_SNAKE_CASE, Warn,
declare_lint!(pub NON_SNAKE_CASE, Warn,
"methods, functions, lifetime parameters and modules should have snake case names")

pub struct NonSnakeCase;
Expand Down Expand Up @@ -930,8 +930,8 @@ impl LintPass for NonSnakeCase {
self.check_snake_case(cx, "trait method", t.ident, t.span);
}

fn check_lifetime_decl(&mut self, cx: &Context, t: &ast::Lifetime) {
self.check_snake_case(cx, "lifetime", t.name.ident(), t.span);
fn check_lifetime_decl(&mut self, cx: &Context, t: &ast::LifetimeDef) {
self.check_snake_case(cx, "lifetime", t.lifetime.name.ident(), t.lifetime.span);
}

fn check_pat(&mut self, cx: &Context, p: &ast::Pat) {
Expand Down Expand Up @@ -962,7 +962,7 @@ impl LintPass for NonSnakeCase {
}
}

declare_lint!(NON_UPPERCASE_STATICS, Allow,
declare_lint!(pub NON_UPPERCASE_STATICS, Allow,
"static constants should have uppercase identifiers")

pub struct NonUppercaseStatics;
Expand Down Expand Up @@ -1143,7 +1143,7 @@ impl LintPass for UnsafeBlock {
}
}

declare_lint!(UNUSED_MUT, Warn,
declare_lint!(pub UNUSED_MUT, Warn,
"detect mut variables which don't need to be mutable")

pub struct UnusedMut;
Expand Down
102 changes: 82 additions & 20 deletions src/librustc/lint/context.rs
Expand Up @@ -66,6 +66,10 @@ pub struct LintStore {

/// Current levels of each lint, and where they were set.
levels: HashMap<LintId, LevelSource>,

/// Map of registered lint groups to what lints they expand to. The bool
/// is true if the lint group was added by a plugin.
lint_groups: HashMap<&'static str, (Vec<LintId>, bool)>,
}

impl LintStore {
Expand All @@ -90,13 +94,18 @@ impl LintStore {
passes: Some(vec!()),
by_name: HashMap::new(),
levels: HashMap::new(),
lint_groups: HashMap::new(),
}
}

pub fn get_lints<'t>(&'t self) -> &'t [(&'static Lint, bool)] {
self.lints.as_slice()
}

pub fn get_lint_groups<'t>(&'t self) -> Vec<(&'static str, Vec<LintId>, bool)> {
self.lint_groups.iter().map(|(k, &(ref v, b))| (*k, v.clone(), b)).collect()
}

pub fn register_pass(&mut self, sess: Option<&Session>,
from_plugin: bool, pass: LintPassObject) {
for &lint in pass.get_lints().iter() {
Expand All @@ -123,6 +132,25 @@ impl LintStore {
self.passes.get_mut_ref().push(pass);
}

pub fn register_group(&mut self, sess: Option<&Session>,
from_plugin: bool, name: &'static str,
to: Vec<LintId>) {
let new = self.lint_groups.insert(name, (to, from_plugin));

if !new {
let msg = format!("duplicate specification of lint group {}", name);
match (sess, from_plugin) {
// We load builtin lints first, so a duplicate is a compiler bug.
// Use early_error when handling -W help with no crate.
(None, _) => early_error(msg.as_slice()),
(Some(sess), false) => sess.bug(msg.as_slice()),

// A duplicate name from a plugin is a user error.
(Some(sess), true) => sess.err(msg.as_slice()),
}
}
}

pub fn register_builtin(&mut self, sess: Option<&Session>) {
macro_rules! add_builtin ( ( $sess:ident, $($name:ident),*, ) => (
{$(
Expand All @@ -136,6 +164,10 @@ impl LintStore {
)*}
))

macro_rules! add_lint_group ( ( $sess:ident, $name:expr, $($lint:ident),* ) => (
self.register_group($sess, false, $name, vec![$(LintId::of(builtin::$lint)),*]);
))

add_builtin!(sess,
HardwiredLints,
WhileTrue,
Expand All @@ -162,6 +194,13 @@ impl LintStore {
MissingDoc,
)

add_lint_group!(sess, "bad_style",
NON_CAMEL_CASE_TYPES, NON_SNAKE_CASE, NON_UPPERCASE_STATICS)

add_lint_group!(sess, "unused",
UNUSED_IMPORTS, UNUSED_VARIABLE, DEAD_ASSIGNMENT, DEAD_CODE,
UNUSED_MUT, UNREACHABLE_CODE)

// We have one lint pass defined in this module.
self.register_pass(sess, false, box GatherNodeLevels as LintPassObject);
}
Expand All @@ -170,8 +209,20 @@ impl LintStore {
for &(ref lint_name, level) in sess.opts.lint_opts.iter() {
match self.by_name.find_equiv(&lint_name.as_slice()) {
Some(&lint_id) => self.set_level(lint_id, (level, CommandLine)),
None => sess.err(format!("unknown {} flag: {}",
level.as_str(), lint_name).as_slice()),
None => {
match self.lint_groups.iter().map(|(&x, &(ref y, _))| (x, y.clone()))
.collect::<HashMap<&'static str, Vec<LintId>>>()
.find_equiv(&lint_name.as_slice()) {
Some(v) => {
v.iter()
.map(|lint_id: &LintId|
self.set_level(*lint_id, (level, CommandLine)))
.collect::<Vec<()>>();
}
None => sess.err(format!("unknown {} flag: {}",
level.as_str(), lint_name).as_slice()),
}
}
}
}
}
Expand Down Expand Up @@ -305,7 +356,7 @@ impl<'a> Context<'a> {
krate: krate,
exported_items: exported_items,
lints: lint_store,
level_stack: vec!(),
level_stack: vec![],
node_levels: RefCell::new(HashMap::new()),
}
}
Expand Down Expand Up @@ -359,35 +410,46 @@ impl<'a> Context<'a> {
let mut pushed = 0u;

for result in gather_attrs(attrs).move_iter() {
let (lint_id, level, span) = match result {
let v = match result {
Err(span) => {
self.tcx.sess.span_err(span, "malformed lint attribute");
continue;
}
Ok((lint_name, level, span)) => {
match self.lints.by_name.find_equiv(&lint_name.get()) {
Some(&lint_id) => (lint_id, level, span),
Some(&lint_id) => vec![(lint_id, level, span)],
None => {
self.span_lint(builtin::UNRECOGNIZED_LINT, span,
format!("unknown `{}` attribute: `{}`",
level.as_str(), lint_name).as_slice());
continue;
match self.lints.lint_groups.find_equiv(&lint_name.get()) {
Some(&(ref v, _)) => v.iter()
.map(|lint_id: &LintId|
(*lint_id, level, span))
.collect(),
None => {
self.span_lint(builtin::UNRECOGNIZED_LINT, span,
format!("unknown `{}` attribute: `{}`",
level.as_str(), lint_name).as_slice());
continue;
}
}
}
}
}
};

let now = self.lints.get_level_source(lint_id).val0();
if now == Forbid && level != Forbid {
let lint_name = lint_id.as_str();
self.tcx.sess.span_err(span,
format!("{}({}) overruled by outer forbid({})",
level.as_str(), lint_name, lint_name).as_slice());
} else if now != level {
let src = self.lints.get_level_source(lint_id).val1();
self.level_stack.push((lint_id, (now, src)));
pushed += 1;
self.lints.set_level(lint_id, (level, Node(span)));
for (lint_id, level, span) in v.move_iter() {
let now = self.lints.get_level_source(lint_id).val0();
if now == Forbid && level != Forbid {
let lint_name = lint_id.as_str();
self.tcx.sess.span_err(span,
format!("{}({}) overruled by outer forbid({})",
level.as_str(), lint_name,
lint_name).as_slice());
} else if now != level {
let src = self.lints.get_level_source(lint_id).val1();
self.level_stack.push((lint_id, (now, src)));
pushed += 1;
self.lints.set_level(lint_id, (level, Node(span)));
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/typeck/infer/equate.rs
Expand Up @@ -27,7 +27,7 @@ pub struct Equate<'f> {
fields: CombineFields<'f>
}

#[allow(non_snake_case_functions)]
#[allow(non_snake_case)]
pub fn Equate<'f>(cf: CombineFields<'f>) -> Equate<'f> {
Equate { fields: cf }
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/typeck/infer/glb.rs
Expand Up @@ -34,7 +34,7 @@ pub struct Glb<'f> {
fields: CombineFields<'f>
}

#[allow(non_snake_case_functions)]
#[allow(non_snake_case)]
pub fn Glb<'f>(cf: CombineFields<'f>) -> Glb<'f> {
Glb { fields: cf }
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/typeck/infer/lub.rs
Expand Up @@ -33,7 +33,7 @@ pub struct Lub<'f> {
fields: CombineFields<'f>
}

#[allow(non_snake_case_functions)]
#[allow(non_snake_case)]
pub fn Lub<'f>(cf: CombineFields<'f>) -> Lub<'f> {
Lub { fields: cf }
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/typeck/infer/sub.rs
Expand Up @@ -32,7 +32,7 @@ pub struct Sub<'f> {
fields: CombineFields<'f>
}

#[allow(non_snake_case_functions)]
#[allow(non_snake_case)]
pub fn Sub<'f>(cf: CombineFields<'f>) -> Sub<'f> {
Sub { fields: cf }
}
Expand Down

5 comments on commit ed2aad8

@bors
Copy link
Contributor

@bors bors commented on ed2aad8 Aug 29, 2014

Choose a reason for hiding this comment

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

saw approval from alexcrichton
at ftxqxd@ed2aad8

@bors
Copy link
Contributor

@bors bors commented on ed2aad8 Aug 29, 2014

Choose a reason for hiding this comment

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

merging P1start/rust/style-lints = ed2aad8 into auto

@bors
Copy link
Contributor

@bors bors commented on ed2aad8 Aug 29, 2014

Choose a reason for hiding this comment

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

P1start/rust/style-lints = ed2aad8 merged ok, testing candidate = 5419b2c

@bors
Copy link
Contributor

@bors bors commented on ed2aad8 Aug 30, 2014

Choose a reason for hiding this comment

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

fast-forwarding master to auto = 5419b2c

Please sign in to comment.