Skip to content

Make bad_style a silent alias for nonstandard_style #54251

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 3 commits into from
Oct 16, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 85 additions & 32 deletions src/librustc/lint/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,8 @@ pub struct LintStore {
/// Lints indexed by name.
by_name: FxHashMap<String, TargetLint>,

/// Map of registered lint groups to what lints they expand to. The first
/// bool is true if the lint group was added by a plugin. The optional string
/// is used to store the new names of deprecated lint group names.
lint_groups: FxHashMap<&'static str, (Vec<LintId>, bool, Option<&'static str>)>,
/// Map of registered lint groups to what lints they expand to.
lint_groups: FxHashMap<&'static str, LintGroup>,

/// Extra info for future incompatibility lints, describing the
/// issue or RFC that caused the incompatibility.
Expand Down Expand Up @@ -127,6 +125,18 @@ pub enum FindLintError {
Removed,
}

struct LintAlias {
name: &'static str,
/// Whether deprecation warnings should be suppressed for this alias.
silent: bool,
}

struct LintGroup {
lint_ids: Vec<LintId>,
from_plugin: bool,
depr: Option<LintAlias>,
}

pub enum CheckLintNameResult<'a> {
Ok(&'a [LintId]),
/// Lint doesn't exist
Expand Down Expand Up @@ -160,9 +170,15 @@ impl LintStore {
}

pub fn get_lint_groups<'t>(&'t self) -> Vec<(&'static str, Vec<LintId>, bool)> {
self.lint_groups.iter().map(|(k, v)| (*k,
v.0.clone(),
v.1)).collect()
self.lint_groups.iter()
.filter(|(_, LintGroup { depr, .. })| {
// Don't display deprecated lint groups.
depr.is_none()
})
.map(|(k, LintGroup { lint_ids, from_plugin, .. })| {
(*k, lint_ids.clone(), *from_plugin)
})
.collect()
}

pub fn register_early_pass(&mut self,
Expand Down Expand Up @@ -245,6 +261,18 @@ impl LintStore {
self.future_incompatible.get(&id)
}

pub fn register_group_alias(
&mut self,
lint_name: &'static str,
alias: &'static str,
) {
self.lint_groups.insert(alias, LintGroup {
lint_ids: vec![],
from_plugin: false,
depr: Some(LintAlias { name: lint_name, silent: true }),
});
}

pub fn register_group(
&mut self,
sess: Option<&Session>,
Expand All @@ -255,11 +283,18 @@ impl LintStore {
) {
let new = self
.lint_groups
.insert(name, (to, from_plugin, None))
.insert(name, LintGroup {
lint_ids: to,
from_plugin,
depr: None,
})
.is_none();
if let Some(deprecated) = deprecated_name {
self.lint_groups
.insert(deprecated, (vec![], from_plugin, Some(name)));
self.lint_groups.insert(deprecated, LintGroup {
lint_ids: vec![],
from_plugin,
depr: Some(LintAlias { name, silent: false }),
});
}

if !new {
Expand Down Expand Up @@ -288,7 +323,7 @@ impl LintStore {
self.by_name.insert(name.into(), Removed(reason.into()));
}

pub fn find_lints(&self, lint_name: &str) -> Result<Vec<LintId>, FindLintError> {
pub fn find_lints(&self, mut lint_name: &str) -> Result<Vec<LintId>, FindLintError> {
match self.by_name.get(lint_name) {
Some(&Id(lint_id)) => Ok(vec![lint_id]),
Some(&Renamed(_, lint_id)) => {
Expand All @@ -298,9 +333,17 @@ impl LintStore {
Err(FindLintError::Removed)
},
None => {
match self.lint_groups.get(lint_name) {
Some(v) => Ok(v.0.clone()),
None => Err(FindLintError::Removed)
loop {
return match self.lint_groups.get(lint_name) {
Some(LintGroup {lint_ids, depr, .. }) => {
if let Some(LintAlias { name, .. }) = depr {
lint_name = name;
continue;
}
Ok(lint_ids.clone())
}
None => Err(FindLintError::Removed)
};
}
}
}
Expand Down Expand Up @@ -366,7 +409,9 @@ impl LintStore {
match self.by_name.get(&complete_name) {
None => match self.lint_groups.get(&*complete_name) {
None => return CheckLintNameResult::Tool(Err((None, String::new()))),
Some(ids) => return CheckLintNameResult::Tool(Ok(&ids.0)),
Some(LintGroup { lint_ids, .. }) => {
return CheckLintNameResult::Tool(Ok(&lint_ids));
}
},
Some(&Id(ref id)) => return CheckLintNameResult::Tool(Ok(slice::from_ref(id))),
// If the lint was registered as removed or renamed by the lint tool, we don't need
Expand All @@ -390,16 +435,20 @@ impl LintStore {
// If neither the lint, nor the lint group exists check if there is a `clippy::`
// variant of this lint
None => self.check_tool_name_for_backwards_compat(&complete_name, "clippy"),
Some(ids) => {
Some(LintGroup { lint_ids, depr, .. }) => {
// Check if the lint group name is deprecated
if let Some(new_name) = ids.2 {
let lint_ids = self.lint_groups.get(new_name).unwrap();
return CheckLintNameResult::Tool(Err((
Some(&lint_ids.0),
new_name.to_string(),
)));
if let Some(LintAlias { name, silent }) = depr {
let LintGroup { lint_ids, .. } = self.lint_groups.get(name).unwrap();
return if *silent {
CheckLintNameResult::Ok(&lint_ids)
} else {
CheckLintNameResult::Tool(Err((
Some(&lint_ids),
name.to_string(),
)))
};
}
CheckLintNameResult::Ok(&ids.0)
CheckLintNameResult::Ok(&lint_ids)
}
},
Some(&Id(ref id)) => CheckLintNameResult::Ok(slice::from_ref(id)),
Expand All @@ -416,16 +465,20 @@ impl LintStore {
None => match self.lint_groups.get(&*complete_name) {
// Now we are sure, that this lint exists nowhere
None => CheckLintNameResult::NoLint,
Some(ids) => {
// Reaching this would be weird, but lets cover this case anyway
if let Some(new_name) = ids.2 {
let lint_ids = self.lint_groups.get(new_name).unwrap();
return CheckLintNameResult::Tool(Err((
Some(&lint_ids.0),
new_name.to_string(),
)));
Some(LintGroup { lint_ids, depr, .. }) => {
// Reaching this would be weird, but let's cover this case anyway
if let Some(LintAlias { name, silent }) = depr {
let LintGroup { lint_ids, .. } = self.lint_groups.get(name).unwrap();
return if *silent {
CheckLintNameResult::Tool(Err((Some(&lint_ids), complete_name)))
} else {
CheckLintNameResult::Tool(Err((
Some(&lint_ids),
name.to_string(),
)))
};
}
CheckLintNameResult::Tool(Err((Some(&ids.0), complete_name)))
CheckLintNameResult::Tool(Err((Some(&lint_ids), complete_name)))
}
},
Some(&Id(ref id)) => {
Expand Down
22 changes: 9 additions & 13 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,30 +82,30 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
($sess:ident, $($name:ident),*,) => (
{$(
store.register_early_pass($sess, false, box $name);
)*}
)
)*}
)
}

macro_rules! add_pre_expansion_builtin {
($sess:ident, $($name:ident),*,) => (
{$(
store.register_pre_expansion_pass($sess, box $name);
)*}
)
)*}
)
}

macro_rules! add_early_builtin_with_new {
($sess:ident, $($name:ident),*,) => (
{$(
store.register_early_pass($sess, false, box $name::new());
)*}
)
)*}
)
}

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

add_pre_expansion_builtin!(sess,
Expand Down Expand Up @@ -159,12 +159,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {

store.register_late_pass(sess, false, box BuiltinCombinedLateLintPass::new());

add_lint_group!(sess,
"bad_style",
NON_CAMEL_CASE_TYPES,
NON_SNAKE_CASE,
NON_UPPER_CASE_GLOBALS);

add_lint_group!(sess,
"nonstandard_style",
NON_CAMEL_CASE_TYPES,
Expand Down Expand Up @@ -353,6 +347,8 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
store.register_removed("unsigned_negation", "replaced by negate_unsigned feature gate");
store.register_removed("negate_unsigned", "cast a signed value instead");
store.register_removed("raw_pointer_derive", "using derive with raw pointers is ok");
// Register lint group aliases
store.register_group_alias("nonstandard_style", "bad_style");
// This was renamed to raw_pointer_derive, which was then removed,
// so it is also considered removed
store.register_removed("raw_pointer_deriving", "using derive with raw pointers is ok");
Expand Down