Auto merge of #16757 - Veykril:style-lints, r=Veykril

fix: Put style lints behind disabled-by-default config

Fixes https://github.com/rust-lang/rust-analyzer/issues/16542
Fixes https://github.com/rust-lang/rust-analyzer/issues/16725
cc https://github.com/rust-lang/rust-analyzer/issues/16628

Our diagnostic infra is not yet setup for those kinds of diagnostics
This commit is contained in:
bors 2024-03-05 10:45:05 +00:00
commit 676455f911
7 changed files with 66 additions and 20 deletions

View file

@ -60,12 +60,17 @@ pub enum BodyValidationDiagnostic {
} }
impl BodyValidationDiagnostic { impl BodyValidationDiagnostic {
pub fn collect(db: &dyn HirDatabase, owner: DefWithBodyId) -> Vec<BodyValidationDiagnostic> { pub fn collect(
db: &dyn HirDatabase,
owner: DefWithBodyId,
validate_lints: bool,
) -> Vec<BodyValidationDiagnostic> {
let _p = let _p =
tracing::span!(tracing::Level::INFO, "BodyValidationDiagnostic::collect").entered(); tracing::span!(tracing::Level::INFO, "BodyValidationDiagnostic::collect").entered();
let infer = db.infer(owner); let infer = db.infer(owner);
let body = db.body(owner); let body = db.body(owner);
let mut validator = ExprValidator { owner, body, infer, diagnostics: Vec::new() }; let mut validator =
ExprValidator { owner, body, infer, diagnostics: Vec::new(), validate_lints };
validator.validate_body(db); validator.validate_body(db);
validator.diagnostics validator.diagnostics
} }
@ -76,6 +81,7 @@ struct ExprValidator {
body: Arc<Body>, body: Arc<Body>,
infer: Arc<InferenceResult>, infer: Arc<InferenceResult>,
diagnostics: Vec<BodyValidationDiagnostic>, diagnostics: Vec<BodyValidationDiagnostic>,
validate_lints: bool,
} }
impl ExprValidator { impl ExprValidator {
@ -139,6 +145,9 @@ impl ExprValidator {
expr: &Expr, expr: &Expr,
filter_map_next_checker: &mut Option<FilterMapNextChecker>, filter_map_next_checker: &mut Option<FilterMapNextChecker>,
) { ) {
if !self.validate_lints {
return;
}
// Check that the number of arguments matches the number of parameters. // Check that the number of arguments matches the number of parameters.
if self.infer.expr_type_mismatches().next().is_some() { if self.infer.expr_type_mismatches().next().is_some() {
@ -308,6 +317,9 @@ impl ExprValidator {
} }
fn check_for_trailing_return(&mut self, body_expr: ExprId, body: &Body) { fn check_for_trailing_return(&mut self, body_expr: ExprId, body: &Body) {
if !self.validate_lints {
return;
}
match &body.exprs[body_expr] { match &body.exprs[body_expr] {
Expr::Block { statements, tail, .. } => { Expr::Block { statements, tail, .. } => {
let last_stmt = tail.or_else(|| match statements.last()? { let last_stmt = tail.or_else(|| match statements.last()? {
@ -340,6 +352,9 @@ impl ExprValidator {
} }
fn check_for_unnecessary_else(&mut self, id: ExprId, expr: &Expr, db: &dyn HirDatabase) { fn check_for_unnecessary_else(&mut self, id: ExprId, expr: &Expr, db: &dyn HirDatabase) {
if !self.validate_lints {
return;
}
if let Expr::If { condition: _, then_branch, else_branch } = expr { if let Expr::If { condition: _, then_branch, else_branch } = expr {
if else_branch.is_none() { if else_branch.is_none() {
return; return;

View file

@ -365,7 +365,7 @@ impl ModuleDef {
Some(name) Some(name)
} }
pub fn diagnostics(self, db: &dyn HirDatabase) -> Vec<AnyDiagnostic> { pub fn diagnostics(self, db: &dyn HirDatabase, style_lints: bool) -> Vec<AnyDiagnostic> {
let id = match self { let id = match self {
ModuleDef::Adt(it) => match it { ModuleDef::Adt(it) => match it {
Adt::Struct(it) => it.id.into(), Adt::Struct(it) => it.id.into(),
@ -387,7 +387,7 @@ impl ModuleDef {
match self.as_def_with_body() { match self.as_def_with_body() {
Some(def) => { Some(def) => {
def.diagnostics(db, &mut acc); def.diagnostics(db, &mut acc, style_lints);
} }
None => { None => {
for diag in hir_ty::diagnostics::incorrect_case(db, id) { for diag in hir_ty::diagnostics::incorrect_case(db, id) {
@ -541,7 +541,12 @@ impl Module {
} }
/// Fills `acc` with the module's diagnostics. /// Fills `acc` with the module's diagnostics.
pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) { pub fn diagnostics(
self,
db: &dyn HirDatabase,
acc: &mut Vec<AnyDiagnostic>,
style_lints: bool,
) {
let name = self.name(db); let name = self.name(db);
let _p = tracing::span!(tracing::Level::INFO, "Module::diagnostics", ?name); let _p = tracing::span!(tracing::Level::INFO, "Module::diagnostics", ?name);
let def_map = self.id.def_map(db.upcast()); let def_map = self.id.def_map(db.upcast());
@ -558,9 +563,9 @@ impl Module {
ModuleDef::Module(m) => { ModuleDef::Module(m) => {
// Only add diagnostics from inline modules // Only add diagnostics from inline modules
if def_map[m.id.local_id].origin.is_inline() { if def_map[m.id.local_id].origin.is_inline() {
m.diagnostics(db, acc) m.diagnostics(db, acc, style_lints)
} }
acc.extend(def.diagnostics(db)) acc.extend(def.diagnostics(db, style_lints))
} }
ModuleDef::Trait(t) => { ModuleDef::Trait(t) => {
for diag in db.trait_data_with_diagnostics(t.id).1.iter() { for diag in db.trait_data_with_diagnostics(t.id).1.iter() {
@ -568,10 +573,10 @@ impl Module {
} }
for item in t.items(db) { for item in t.items(db) {
item.diagnostics(db, acc); item.diagnostics(db, acc, style_lints);
} }
acc.extend(def.diagnostics(db)) acc.extend(def.diagnostics(db, style_lints))
} }
ModuleDef::Adt(adt) => { ModuleDef::Adt(adt) => {
match adt { match adt {
@ -587,17 +592,17 @@ impl Module {
} }
Adt::Enum(e) => { Adt::Enum(e) => {
for v in e.variants(db) { for v in e.variants(db) {
acc.extend(ModuleDef::Variant(v).diagnostics(db)); acc.extend(ModuleDef::Variant(v).diagnostics(db, style_lints));
for diag in db.enum_variant_data_with_diagnostics(v.id).1.iter() { for diag in db.enum_variant_data_with_diagnostics(v.id).1.iter() {
emit_def_diagnostic(db, acc, diag); emit_def_diagnostic(db, acc, diag);
} }
} }
} }
} }
acc.extend(def.diagnostics(db)) acc.extend(def.diagnostics(db, style_lints))
} }
ModuleDef::Macro(m) => emit_macro_def_diagnostics(db, acc, m), ModuleDef::Macro(m) => emit_macro_def_diagnostics(db, acc, m),
_ => acc.extend(def.diagnostics(db)), _ => acc.extend(def.diagnostics(db, style_lints)),
} }
} }
self.legacy_macros(db).into_iter().for_each(|m| emit_macro_def_diagnostics(db, acc, m)); self.legacy_macros(db).into_iter().for_each(|m| emit_macro_def_diagnostics(db, acc, m));
@ -738,7 +743,7 @@ impl Module {
} }
for &item in &db.impl_data(impl_def.id).items { for &item in &db.impl_data(impl_def.id).items {
AssocItem::from(item).diagnostics(db, acc); AssocItem::from(item).diagnostics(db, acc, style_lints);
} }
} }
} }
@ -1616,14 +1621,19 @@ impl DefWithBody {
} }
} }
pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) { pub fn diagnostics(
self,
db: &dyn HirDatabase,
acc: &mut Vec<AnyDiagnostic>,
style_lints: bool,
) {
db.unwind_if_cancelled(); db.unwind_if_cancelled();
let krate = self.module(db).id.krate(); let krate = self.module(db).id.krate();
let (body, source_map) = db.body_with_source_map(self.into()); let (body, source_map) = db.body_with_source_map(self.into());
for (_, def_map) in body.blocks(db.upcast()) { for (_, def_map) in body.blocks(db.upcast()) {
Module { id: def_map.module_id(DefMap::ROOT) }.diagnostics(db, acc); Module { id: def_map.module_id(DefMap::ROOT) }.diagnostics(db, acc, style_lints);
} }
for diag in source_map.diagnostics() { for diag in source_map.diagnostics() {
@ -1784,7 +1794,7 @@ impl DefWithBody {
} }
} }
for diagnostic in BodyValidationDiagnostic::collect(db, self.into()) { for diagnostic in BodyValidationDiagnostic::collect(db, self.into(), style_lints) {
acc.extend(AnyDiagnostic::body_validation_diagnostic(db, diagnostic, &source_map)); acc.extend(AnyDiagnostic::body_validation_diagnostic(db, diagnostic, &source_map));
} }
@ -2897,13 +2907,18 @@ impl AssocItem {
} }
} }
pub fn diagnostics(self, db: &dyn HirDatabase, acc: &mut Vec<AnyDiagnostic>) { pub fn diagnostics(
self,
db: &dyn HirDatabase,
acc: &mut Vec<AnyDiagnostic>,
style_lints: bool,
) {
match self { match self {
AssocItem::Function(func) => { AssocItem::Function(func) => {
DefWithBody::from(func).diagnostics(db, acc); DefWithBody::from(func).diagnostics(db, acc, style_lints);
} }
AssocItem::Const(const_) => { AssocItem::Const(const_) => {
DefWithBody::from(const_).diagnostics(db, acc); DefWithBody::from(const_).diagnostics(db, acc, style_lints);
} }
AssocItem::TypeAlias(type_alias) => { AssocItem::TypeAlias(type_alias) => {
for diag in hir_ty::diagnostics::incorrect_case(db, type_alias.id.into()) { for diag in hir_ty::diagnostics::incorrect_case(db, type_alias.id.into()) {

View file

@ -227,6 +227,7 @@ pub struct DiagnosticsConfig {
pub disable_experimental: bool, pub disable_experimental: bool,
pub disabled: FxHashSet<String>, pub disabled: FxHashSet<String>,
pub expr_fill_default: ExprFillDefaultMode, pub expr_fill_default: ExprFillDefaultMode,
pub style_lints: bool,
// FIXME: We may want to include a whole `AssistConfig` here // FIXME: We may want to include a whole `AssistConfig` here
pub insert_use: InsertUseConfig, pub insert_use: InsertUseConfig,
pub prefer_no_std: bool, pub prefer_no_std: bool,
@ -245,6 +246,7 @@ impl DiagnosticsConfig {
disable_experimental: Default::default(), disable_experimental: Default::default(),
disabled: Default::default(), disabled: Default::default(),
expr_fill_default: Default::default(), expr_fill_default: Default::default(),
style_lints: true,
insert_use: InsertUseConfig { insert_use: InsertUseConfig {
granularity: ImportGranularity::Preserve, granularity: ImportGranularity::Preserve,
enforce_granularity: false, enforce_granularity: false,
@ -324,7 +326,7 @@ pub fn diagnostics(
let mut diags = Vec::new(); let mut diags = Vec::new();
if let Some(m) = module { if let Some(m) = module {
m.diagnostics(db, &mut diags); m.diagnostics(db, &mut diags, config.style_lints);
} }
for diag in diags { for diag in diags {

View file

@ -982,6 +982,7 @@ impl flags::AnalysisStats {
}, },
prefer_no_std: false, prefer_no_std: false,
prefer_prelude: true, prefer_prelude: true,
style_lints: false,
}, },
ide::AssistResolveStrategy::All, ide::AssistResolveStrategy::All,
file_id, file_id,

View file

@ -311,6 +311,8 @@ config_data! {
/// Map of prefixes to be substituted when parsing diagnostic file paths. /// Map of prefixes to be substituted when parsing diagnostic file paths.
/// This should be the reverse mapping of what is passed to `rustc` as `--remap-path-prefix`. /// This should be the reverse mapping of what is passed to `rustc` as `--remap-path-prefix`.
diagnostics_remapPrefix: FxHashMap<String, String> = "{}", diagnostics_remapPrefix: FxHashMap<String, String> = "{}",
/// Whether to run additional style lints.
diagnostics_styleLints_enable: bool = "false",
/// List of warnings that should be displayed with hint severity. /// List of warnings that should be displayed with hint severity.
/// ///
/// The warnings will be indicated by faded text or three dots in code /// The warnings will be indicated by faded text or three dots in code
@ -1162,6 +1164,7 @@ impl Config {
insert_use: self.insert_use_config(), insert_use: self.insert_use_config(),
prefer_no_std: self.data.imports_preferNoStd, prefer_no_std: self.data.imports_preferNoStd,
prefer_prelude: self.data.imports_preferPrelude, prefer_prelude: self.data.imports_preferPrelude,
style_lints: self.data.diagnostics_styleLints_enable,
} }
} }

View file

@ -386,6 +386,11 @@ have more false positives than usual.
Map of prefixes to be substituted when parsing diagnostic file paths. Map of prefixes to be substituted when parsing diagnostic file paths.
This should be the reverse mapping of what is passed to `rustc` as `--remap-path-prefix`. This should be the reverse mapping of what is passed to `rustc` as `--remap-path-prefix`.
-- --
[[rust-analyzer.diagnostics.styleLints.enable]]rust-analyzer.diagnostics.styleLints.enable (default: `false`)::
+
--
Whether to run additional style lints.
--
[[rust-analyzer.diagnostics.warningsAsHint]]rust-analyzer.diagnostics.warningsAsHint (default: `[]`):: [[rust-analyzer.diagnostics.warningsAsHint]]rust-analyzer.diagnostics.warningsAsHint (default: `[]`)::
+ +
-- --

View file

@ -948,6 +948,11 @@
"default": {}, "default": {},
"type": "object" "type": "object"
}, },
"rust-analyzer.diagnostics.styleLints.enable": {
"markdownDescription": "Whether to run additional style lints.",
"default": false,
"type": "boolean"
},
"rust-analyzer.diagnostics.warningsAsHint": { "rust-analyzer.diagnostics.warningsAsHint": {
"markdownDescription": "List of warnings that should be displayed with hint severity.\n\nThe warnings will be indicated by faded text or three dots in code\nand will not show up in the `Problems Panel`.", "markdownDescription": "List of warnings that should be displayed with hint severity.\n\nThe warnings will be indicated by faded text or three dots in code\nand will not show up in the `Problems Panel`.",
"default": [], "default": [],