Skip to content

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

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.

Sign up for GitHub

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
Mar 5, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
19 changes: 17 additions & 2 deletions crates/hir-ty/src/diagnostics/expr.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,17 @@ pub enum 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 =
tracing::span!(tracing::Level::INFO, "BodyValidationDiagnostic::collect").entered();
let infer = db.infer(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.diagnostics
}
Expand All @@ -76,6 +81,7 @@ struct ExprValidator {
body: Arc<Body>,
infer: Arc<InferenceResult>,
diagnostics: Vec<BodyValidationDiagnostic>,
validate_lints: bool,
}

impl ExprValidator {
Expand Down Expand Up @@ -139,6 +145,9 @@ impl ExprValidator {
expr: &Expr,
filter_map_next_checker: &mut Option<FilterMapNextChecker>,
) {
if !self.validate_lints {
return;
}
// Check that the number of arguments matches the number of parameters.

if self.infer.expr_type_mismatches().next().is_some() {
Expand Down Expand Up @@ -308,6 +317,9 @@ impl ExprValidator {
}

fn check_for_trailing_return(&mut self, body_expr: ExprId, body: &Body) {
if !self.validate_lints {
return;
}
match &body.exprs[body_expr] {
Expr::Block { statements, tail, .. } => {
let last_stmt = tail.or_else(|| match statements.last()? {
Expand Down Expand Up @@ -340,6 +352,9 @@ impl ExprValidator {
}

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 else_branch.is_none() {
return;
Expand Down
49 changes: 32 additions & 17 deletions crates/hir/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ impl ModuleDef {
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 {
ModuleDef::Adt(it) => match it {
Adt::Struct(it) => it.id.into(),
Expand All @@ -387,7 +387,7 @@ impl ModuleDef {

match self.as_def_with_body() {
Some(def) => {
def.diagnostics(db, &mut acc);
def.diagnostics(db, &mut acc, style_lints);
}
None => {
for diag in hir_ty::diagnostics::incorrect_case(db, id) {
Expand Down Expand Up @@ -541,7 +541,12 @@ impl Module {
}

/// 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 _p = tracing::span!(tracing::Level::INFO, "Module::diagnostics", ?name);
let def_map = self.id.def_map(db.upcast());
Expand All @@ -558,20 +563,20 @@ impl Module {
ModuleDef::Module(m) => {
// Only add diagnostics from inline modules
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) => {
for diag in db.trait_data_with_diagnostics(t.id).1.iter() {
emit_def_diagnostic(db, acc, diag);
}

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) => {
match adt {
Expand All @@ -587,17 +592,17 @@ impl Module {
}
Adt::Enum(e) => {
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() {
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),
_ => 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));
Expand Down Expand Up @@ -738,7 +743,7 @@ impl Module {
}

for &item in &db.impl_data(impl_def.id).items {
AssocItem::from(item).diagnostics(db, acc);
AssocItem::from(item).diagnostics(db, acc, style_lints);
}
}
}
Expand Down Expand Up @@ -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();
let krate = self.module(db).id.krate();

let (body, source_map) = db.body_with_source_map(self.into());

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() {
Expand Down Expand Up @@ -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));
}

Expand Down Expand Up @@ -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 {
AssocItem::Function(func) => {
DefWithBody::from(func).diagnostics(db, acc);
DefWithBody::from(func).diagnostics(db, acc, style_lints);
}
AssocItem::Const(const_) => {
DefWithBody::from(const_).diagnostics(db, acc);
DefWithBody::from(const_).diagnostics(db, acc, style_lints);
}
AssocItem::TypeAlias(type_alias) => {
for diag in hir_ty::diagnostics::incorrect_case(db, type_alias.id.into()) {
Expand Down
4 changes: 3 additions & 1 deletion crates/ide-diagnostics/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ pub struct DiagnosticsConfig {
pub disable_experimental: bool,
pub disabled: FxHashSet<String>,
pub expr_fill_default: ExprFillDefaultMode,
pub style_lints: bool,
// FIXME: We may want to include a whole `AssistConfig` here
pub insert_use: InsertUseConfig,
pub prefer_no_std: bool,
Expand All @@ -245,6 +246,7 @@ impl DiagnosticsConfig {
disable_experimental: Default::default(),
disabled: Default::default(),
expr_fill_default: Default::default(),
style_lints: true,
insert_use: InsertUseConfig {
granularity: ImportGranularity::Preserve,
enforce_granularity: false,
Expand Down Expand Up @@ -324,7 +326,7 @@ pub fn diagnostics(

let mut diags = Vec::new();
if let Some(m) = module {
m.diagnostics(db, &mut diags);
m.diagnostics(db, &mut diags, config.style_lints);
}

for diag in diags {
Expand Down
1 change: 1 addition & 0 deletions crates/rust-analyzer/src/cli/analysis_stats.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -982,6 +982,7 @@ impl flags::AnalysisStats {
},
prefer_no_std: false,
prefer_prelude: true,
style_lints: false,
},
ide::AssistResolveStrategy::All,
file_id,
Expand Down
3 changes: 3 additions & 0 deletions crates/rust-analyzer/src/config.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,8 @@ config_data! {
/// 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`.
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.
///
/// The warnings will be indicated by faded text or three dots in code
Expand Down Expand Up @@ -1162,6 +1164,7 @@ impl Config {
insert_use: self.insert_use_config(),
prefer_no_std: self.data.imports_preferNoStd,
prefer_prelude: self.data.imports_preferPrelude,
style_lints: self.data.diagnostics_styleLints_enable,
}
}

Expand Down
5 changes: 5 additions & 0 deletions docs/user/generated_config.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,11 @@ have more false positives than usual.
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`.
--
[[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: `[]`)::
+
--
Expand Down
5 changes: 5 additions & 0 deletions editors/code/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -948,6 +948,11 @@
"default": {},
"type": "object"
},
"rust-analyzer.diagnostics.styleLints.enable": {
"markdownDescription": "Whether to run additional style lints.",
"default": false,
"type": "boolean"
},
"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`.",
"default": [],
Expand Down