From 7731714578d4ae6eb7a54ead2e51ae032e9a700a Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Sat, 12 Jun 2021 22:05:23 +0300 Subject: internal: move diagnostics infra to hir --- crates/hir/src/diagnostics.rs | 35 ++- crates/hir/src/diagnostics_sink.rs | 109 +++++++++ crates/hir/src/lib.rs | 32 +-- crates/hir_ty/src/diagnostics.rs | 156 +----------- crates/hir_ty/src/diagnostics/decl_check.rs | 359 +--------------------------- crates/hir_ty/src/diagnostics_sink.rs | 109 --------- crates/hir_ty/src/lib.rs | 1 - crates/ide/src/diagnostics.rs | 354 +++++++++++++++++++++++++++ 8 files changed, 526 insertions(+), 629 deletions(-) create mode 100644 crates/hir/src/diagnostics_sink.rs delete mode 100644 crates/hir_ty/src/diagnostics_sink.rs diff --git a/crates/hir/src/diagnostics.rs b/crates/hir/src/diagnostics.rs index 26dbcd86a..8a7c3a4fd 100644 --- a/crates/hir/src/diagnostics.rs +++ b/crates/hir/src/diagnostics.rs @@ -11,9 +11,8 @@ use hir_expand::{name::Name, HirFileId, InFile}; use stdx::format_to; use syntax::{ast, AstPtr, SyntaxNodePtr, TextRange}; -pub use hir_ty::{ - diagnostics::IncorrectCase, - diagnostics_sink::{Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder}, +pub use crate::diagnostics_sink::{ + Diagnostic, DiagnosticCode, DiagnosticSink, DiagnosticSinkBuilder, }; // Diagnostic: unresolved-module @@ -578,3 +577,33 @@ impl Diagnostic for InternalBailedOut { self } } + +pub use hir_ty::diagnostics::IncorrectCase; + +impl Diagnostic for IncorrectCase { + fn code(&self) -> DiagnosticCode { + DiagnosticCode("incorrect-ident-case") + } + + fn message(&self) -> String { + format!( + "{} `{}` should have {} name, e.g. `{}`", + self.ident_type, + self.ident_text, + self.expected_case.to_string(), + self.suggested_text + ) + } + + fn display_source(&self) -> InFile { + InFile::new(self.file, self.ident.clone().into()) + } + + fn as_any(&self) -> &(dyn Any + Send + 'static) { + self + } + + fn is_experimental(&self) -> bool { + true + } +} diff --git a/crates/hir/src/diagnostics_sink.rs b/crates/hir/src/diagnostics_sink.rs new file mode 100644 index 000000000..084fa8b06 --- /dev/null +++ b/crates/hir/src/diagnostics_sink.rs @@ -0,0 +1,109 @@ +//! Semantic errors and warnings. +//! +//! The `Diagnostic` trait defines a trait object which can represent any +//! diagnostic. +//! +//! `DiagnosticSink` struct is used as an emitter for diagnostic. When creating +//! a `DiagnosticSink`, you supply a callback which can react to a `dyn +//! Diagnostic` or to any concrete diagnostic (downcasting is used internally). +//! +//! Because diagnostics store file offsets, it's a bad idea to store them +//! directly in salsa. For this reason, every hir subsytem defines it's own +//! strongly-typed closed set of diagnostics which use hir ids internally, are +//! stored in salsa and do *not* implement the `Diagnostic` trait. Instead, a +//! subsystem provides a separate, non-query-based API which can walk all stored +//! values and transform them into instances of `Diagnostic`. + +use std::{any::Any, fmt}; + +use hir_expand::InFile; +use syntax::SyntaxNodePtr; + +#[derive(Copy, Clone, Debug, PartialEq)] +pub struct DiagnosticCode(pub &'static str); + +impl DiagnosticCode { + pub fn as_str(&self) -> &str { + self.0 + } +} + +pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { + fn code(&self) -> DiagnosticCode; + fn message(&self) -> String; + /// Source element that triggered the diagnostics. + /// + /// Note that this should reflect "semantics", rather than specific span we + /// want to highlight. When rendering the diagnostics into an error message, + /// the IDE will fetch the `SyntaxNode` and will narrow the span + /// appropriately. + fn display_source(&self) -> InFile; + fn as_any(&self) -> &(dyn Any + Send + 'static); + fn is_experimental(&self) -> bool { + false + } +} + +pub struct DiagnosticSink<'a> { + callbacks: Vec Result<(), ()> + 'a>>, + filters: Vec bool + 'a>>, + default_callback: Box, +} + +impl<'a> DiagnosticSink<'a> { + pub fn push(&mut self, d: impl Diagnostic) { + let d: &dyn Diagnostic = &d; + self._push(d); + } + + fn _push(&mut self, d: &dyn Diagnostic) { + for filter in &mut self.filters { + if !filter(d) { + return; + } + } + for cb in &mut self.callbacks { + match cb(d) { + Ok(()) => return, + Err(()) => (), + } + } + (self.default_callback)(d) + } +} + +pub struct DiagnosticSinkBuilder<'a> { + callbacks: Vec Result<(), ()> + 'a>>, + filters: Vec bool + 'a>>, +} + +impl<'a> DiagnosticSinkBuilder<'a> { + pub fn new() -> Self { + Self { callbacks: Vec::new(), filters: Vec::new() } + } + + pub fn filter bool + 'a>(mut self, cb: F) -> Self { + self.filters.push(Box::new(cb)); + self + } + + pub fn on(mut self, mut cb: F) -> Self { + let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::() { + Some(d) => { + cb(d); + Ok(()) + } + None => Err(()), + }; + self.callbacks.push(Box::new(cb)); + self + } + + pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { + DiagnosticSink { + callbacks: self.callbacks, + filters: self.filters, + default_callback: Box::new(default_callback), + } + } +} diff --git a/crates/hir/src/lib.rs b/crates/hir/src/lib.rs index dd5515c2b..2468c0dc6 100644 --- a/crates/hir/src/lib.rs +++ b/crates/hir/src/lib.rs @@ -27,6 +27,7 @@ mod attrs; mod has_source; pub mod diagnostics; +pub mod diagnostics_sink; pub mod db; mod display; @@ -35,13 +36,6 @@ use std::{iter, sync::Arc}; use arrayvec::ArrayVec; use base_db::{CrateDisplayName, CrateId, Edition, FileId}; -use diagnostics::{ - BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError, MismatchedArgCount, - MissingFields, MissingOkOrSomeInTailExpr, MissingPatFields, MissingUnsafe, NoSuchField, - RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, UnimplementedBuiltinMacro, - UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, UnresolvedModule, - UnresolvedProcMacro, -}; use either::Either; use hir_def::{ adt::{ReprKind, VariantData}, @@ -64,8 +58,7 @@ use hir_ty::{ consteval::ConstExt, could_unify, diagnostics::BodyValidationDiagnostic, - diagnostics_sink::DiagnosticSink, - method_resolution::{self, def_crates, TyFingerprint}, + method_resolution::{self, TyFingerprint}, primitive::UintTy, subst_prefix, traits::FnTrait, @@ -87,7 +80,14 @@ use tt::{Ident, Leaf, Literal, TokenTree}; use crate::{ db::{DefDatabase, HirDatabase}, - diagnostics::MissingMatchArms, + diagnostics::{ + BreakOutsideOfLoop, InactiveCode, InternalBailedOut, MacroError, MismatchedArgCount, + MissingFields, MissingMatchArms, MissingOkOrSomeInTailExpr, MissingPatFields, + MissingUnsafe, NoSuchField, RemoveThisSemicolon, ReplaceFilterMapNextWithFindMap, + UnimplementedBuiltinMacro, UnresolvedExternCrate, UnresolvedImport, UnresolvedMacroCall, + UnresolvedModule, UnresolvedProcMacro, + }, + diagnostics_sink::DiagnosticSink, }; pub use crate::{ @@ -361,7 +361,9 @@ impl ModuleDef { None => return, }; - hir_ty::diagnostics::validate_module_item(db, module.id.krate(), id, sink) + for diag in hir_ty::diagnostics::validate_module_item(db, module.id.krate(), id) { + sink.push(diag) + } } } @@ -1225,7 +1227,9 @@ impl Function { } } - hir_ty::diagnostics::validate_module_item(db, krate, self.id.into(), sink); + for diag in hir_ty::diagnostics::validate_module_item(db, krate, self.id.into()) { + sink.push(diag) + } } /// Whether this function declaration has a definition. @@ -1944,7 +1948,7 @@ impl Impl { } pub fn all_for_type(db: &dyn HirDatabase, Type { krate, ty, .. }: Type) -> Vec { - let def_crates = match def_crates(db, &ty, krate) { + let def_crates = match method_resolution::def_crates(db, &ty, krate) { Some(def_crates) => def_crates, None => return Vec::new(), }; @@ -2350,7 +2354,7 @@ impl Type { krate: Crate, mut callback: impl FnMut(AssocItem) -> Option, ) -> Option { - for krate in def_crates(db, &self.ty, krate.id)? { + for krate in method_resolution::def_crates(db, &self.ty, krate.id)? { let impls = db.inherent_impls_in_crate(krate); for impl_def in impls.for_self_ty(&self.ty) { diff --git a/crates/hir_ty/src/diagnostics.rs b/crates/hir_ty/src/diagnostics.rs index f3236bc06..407273943 100644 --- a/crates/hir_ty/src/diagnostics.rs +++ b/crates/hir_ty/src/diagnostics.rs @@ -4,17 +4,14 @@ mod match_check; mod unsafe_check; mod decl_check; -use std::{any::Any, fmt}; +use std::fmt; use base_db::CrateId; use hir_def::ModuleDefId; -use hir_expand::{HirFileId, InFile}; -use syntax::{ast, AstPtr, SyntaxNodePtr}; +use hir_expand::HirFileId; +use syntax::{ast, AstPtr}; -use crate::{ - db::HirDatabase, - diagnostics_sink::{Diagnostic, DiagnosticCode, DiagnosticSink}, -}; +use crate::db::HirDatabase; pub use crate::diagnostics::{ expr::{ @@ -27,11 +24,11 @@ pub fn validate_module_item( db: &dyn HirDatabase, krate: CrateId, owner: ModuleDefId, - sink: &mut DiagnosticSink<'_>, -) { +) -> Vec { let _p = profile::span("validate_module_item"); - let mut validator = decl_check::DeclValidator::new(db, krate, sink); + let mut validator = decl_check::DeclValidator::new(db, krate); validator.validate_item(owner); + validator.sink } #[derive(Debug)] @@ -99,142 +96,3 @@ pub struct IncorrectCase { pub ident_text: String, pub suggested_text: String, } - -impl Diagnostic for IncorrectCase { - fn code(&self) -> DiagnosticCode { - DiagnosticCode("incorrect-ident-case") - } - - fn message(&self) -> String { - format!( - "{} `{}` should have {} name, e.g. `{}`", - self.ident_type, - self.ident_text, - self.expected_case.to_string(), - self.suggested_text - ) - } - - fn display_source(&self) -> InFile { - InFile::new(self.file, self.ident.clone().into()) - } - - fn as_any(&self) -> &(dyn Any + Send + 'static) { - self - } - - fn is_experimental(&self) -> bool { - true - } -} - -#[cfg(test)] -mod tests { - use base_db::{fixture::WithFixture, FileId, SourceDatabase, SourceDatabaseExt}; - use hir_def::{db::DefDatabase, AssocItemId, ModuleDefId}; - use hir_expand::db::AstDatabase; - use rustc_hash::FxHashMap; - use syntax::{TextRange, TextSize}; - - use crate::{ - diagnostics::validate_module_item, - diagnostics_sink::{Diagnostic, DiagnosticSinkBuilder}, - test_db::TestDB, - }; - - impl TestDB { - fn diagnostics(&self, mut cb: F) { - let crate_graph = self.crate_graph(); - for krate in crate_graph.iter() { - let crate_def_map = self.crate_def_map(krate); - - let mut fns = Vec::new(); - for (module_id, _) in crate_def_map.modules() { - for decl in crate_def_map[module_id].scope.declarations() { - let mut sink = DiagnosticSinkBuilder::new().build(&mut cb); - validate_module_item(self, krate, decl, &mut sink); - - if let ModuleDefId::FunctionId(f) = decl { - fns.push(f) - } - } - - for impl_id in crate_def_map[module_id].scope.impls() { - let impl_data = self.impl_data(impl_id); - for item in impl_data.items.iter() { - if let AssocItemId::FunctionId(f) = item { - let mut sink = DiagnosticSinkBuilder::new().build(&mut cb); - validate_module_item( - self, - krate, - ModuleDefId::FunctionId(*f), - &mut sink, - ); - fns.push(*f) - } - } - } - } - } - } - } - - pub(crate) fn check_diagnostics(ra_fixture: &str) { - let db = TestDB::with_files(ra_fixture); - let annotations = db.extract_annotations(); - - let mut actual: FxHashMap> = FxHashMap::default(); - db.diagnostics(|d| { - let src = d.display_source(); - let root = db.parse_or_expand(src.file_id).unwrap(); - // FIXME: macros... - let file_id = src.file_id.original_file(&db); - let range = src.value.to_node(&root).text_range(); - let message = d.message(); - actual.entry(file_id).or_default().push((range, message)); - }); - - for (file_id, diags) in actual.iter_mut() { - diags.sort_by_key(|it| it.0.start()); - let text = db.file_text(*file_id); - // For multiline spans, place them on line start - for (range, content) in diags { - if text[*range].contains('\n') { - *range = TextRange::new(range.start(), range.start() + TextSize::from(1)); - *content = format!("... {}", content); - } - } - } - - assert_eq!(annotations, actual); - } - - #[test] - fn import_extern_crate_clash_with_inner_item() { - // This is more of a resolver test, but doesn't really work with the hir_def testsuite. - - check_diagnostics( - r#" -//- /lib.rs crate:lib deps:jwt -mod permissions; - -use permissions::jwt; - -fn f() { - fn inner() {} - jwt::Claims {}; // should resolve to the local one with 0 fields, and not get a diagnostic -} - -//- /permissions.rs -pub mod jwt { - pub struct Claims {} -} - -//- /jwt/lib.rs crate:jwt -pub struct Claims { - field: u8, -} - "#, - ); - } -} diff --git a/crates/hir_ty/src/diagnostics/decl_check.rs b/crates/hir_ty/src/diagnostics/decl_check.rs index cfb5d7320..ca452e879 100644 --- a/crates/hir_ty/src/diagnostics/decl_check.rs +++ b/crates/hir_ty/src/diagnostics/decl_check.rs @@ -29,7 +29,6 @@ use syntax::{ use crate::{ db::HirDatabase, diagnostics::{decl_check::case_conv::*, CaseType, IdentType, IncorrectCase}, - diagnostics_sink::DiagnosticSink, }; mod allow { @@ -40,10 +39,10 @@ mod allow { pub(super) const NON_CAMEL_CASE_TYPES: &str = "non_camel_case_types"; } -pub(super) struct DeclValidator<'a, 'b> { +pub(super) struct DeclValidator<'a> { db: &'a dyn HirDatabase, krate: CrateId, - sink: &'a mut DiagnosticSink<'b>, + pub(super) sink: Vec, } #[derive(Debug)] @@ -53,13 +52,9 @@ struct Replacement { expected_case: CaseType, } -impl<'a, 'b> DeclValidator<'a, 'b> { - pub(super) fn new( - db: &'a dyn HirDatabase, - krate: CrateId, - sink: &'a mut DiagnosticSink<'b>, - ) -> DeclValidator<'a, 'b> { - DeclValidator { db, krate, sink } +impl<'a> DeclValidator<'a> { + pub(super) fn new(db: &'a dyn HirDatabase, krate: CrateId) -> DeclValidator<'a> { + DeclValidator { db, krate, sink: Vec::new() } } pub(super) fn validate_item(&mut self, item: ModuleDefId) { @@ -121,7 +116,6 @@ impl<'a, 'b> DeclValidator<'a, 'b> { fn validate_func(&mut self, func: FunctionId) { let data = self.db.function_data(func); if data.is_in_extern_block() { - cov_mark::hit!(extern_func_incorrect_case_ignored); return; } @@ -131,7 +125,7 @@ impl<'a, 'b> DeclValidator<'a, 'b> { for (_, block_def_map) in body.blocks(self.db.upcast()) { for (_, module) in block_def_map.modules() { for def_id in module.scope.declarations() { - let mut validator = DeclValidator::new(self.db, self.krate, self.sink); + let mut validator = DeclValidator::new(self.db, self.krate); validator.validate_item(def_id); } } @@ -578,7 +572,6 @@ impl<'a, 'b> DeclValidator<'a, 'b> { fn validate_static(&mut self, static_id: StaticId) { let data = self.db.static_data(static_id); if data.is_extern { - cov_mark::hit!(extern_static_incorrect_case_ignored); return; } @@ -623,343 +616,3 @@ impl<'a, 'b> DeclValidator<'a, 'b> { self.sink.push(diagnostic); } } - -#[cfg(test)] -mod tests { - use crate::diagnostics::tests::check_diagnostics; - - #[test] - fn incorrect_function_name() { - check_diagnostics( - r#" -fn NonSnakeCaseName() {} -// ^^^^^^^^^^^^^^^^ Function `NonSnakeCaseName` should have snake_case name, e.g. `non_snake_case_name` -"#, - ); - } - - #[test] - fn incorrect_function_params() { - check_diagnostics( - r#" -fn foo(SomeParam: u8) {} - // ^^^^^^^^^ Parameter `SomeParam` should have snake_case name, e.g. `some_param` - -fn foo2(ok_param: &str, CAPS_PARAM: u8) {} - // ^^^^^^^^^^ Parameter `CAPS_PARAM` should have snake_case name, e.g. `caps_param` -"#, - ); - } - - #[test] - fn incorrect_variable_names() { - check_diagnostics( - r#" -fn foo() { - let SOME_VALUE = 10; - // ^^^^^^^^^^ Variable `SOME_VALUE` should have snake_case name, e.g. `some_value` - let AnotherValue = 20; - // ^^^^^^^^^^^^ Variable `AnotherValue` should have snake_case name, e.g. `another_value` -} -"#, - ); - } - - #[test] - fn incorrect_struct_names() { - check_diagnostics( - r#" -struct non_camel_case_name {} - // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have CamelCase name, e.g. `NonCamelCaseName` - -struct SCREAMING_CASE {} - // ^^^^^^^^^^^^^^ Structure `SCREAMING_CASE` should have CamelCase name, e.g. `ScreamingCase` -"#, - ); - } - - #[test] - fn no_diagnostic_for_camel_cased_acronyms_in_struct_name() { - check_diagnostics( - r#" -struct AABB {} -"#, - ); - } - - #[test] - fn incorrect_struct_field() { - check_diagnostics( - r#" -struct SomeStruct { SomeField: u8 } - // ^^^^^^^^^ Field `SomeField` should have snake_case name, e.g. `some_field` -"#, - ); - } - - #[test] - fn incorrect_enum_names() { - check_diagnostics( - r#" -enum some_enum { Val(u8) } - // ^^^^^^^^^ Enum `some_enum` should have CamelCase name, e.g. `SomeEnum` - -enum SOME_ENUM - // ^^^^^^^^^ Enum `SOME_ENUM` should have CamelCase name, e.g. `SomeEnum` -"#, - ); - } - - #[test] - fn no_diagnostic_for_camel_cased_acronyms_in_enum_name() { - check_diagnostics( - r#" -enum AABB {} -"#, - ); - } - - #[test] - fn incorrect_enum_variant_name() { - check_diagnostics( - r#" -enum SomeEnum { SOME_VARIANT(u8) } - // ^^^^^^^^^^^^ Variant `SOME_VARIANT` should have CamelCase name, e.g. `SomeVariant` -"#, - ); - } - - #[test] - fn incorrect_const_name() { - check_diagnostics( - r#" -const some_weird_const: u8 = 10; - // ^^^^^^^^^^^^^^^^ Constant `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST` - -fn func() { - const someConstInFunc: &str = "hi there"; - // ^^^^^^^^^^^^^^^ Constant `someConstInFunc` should have UPPER_SNAKE_CASE name, e.g. `SOME_CONST_IN_FUNC` - -} -"#, - ); - } - - #[test] - fn incorrect_static_name() { - check_diagnostics( - r#" -static some_weird_const: u8 = 10; - // ^^^^^^^^^^^^^^^^ Static variable `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST` - -fn func() { - static someConstInFunc: &str = "hi there"; - // ^^^^^^^^^^^^^^^ Static variable `someConstInFunc` should have UPPER_SNAKE_CASE name, e.g. `SOME_CONST_IN_FUNC` -} -"#, - ); - } - - #[test] - fn fn_inside_impl_struct() { - check_diagnostics( - r#" -struct someStruct; - // ^^^^^^^^^^ Structure `someStruct` should have CamelCase name, e.g. `SomeStruct` - -impl someStruct { - fn SomeFunc(&self) { - // ^^^^^^^^ Function `SomeFunc` should have snake_case name, e.g. `some_func` - static someConstInFunc: &str = "hi there"; - // ^^^^^^^^^^^^^^^ Static variable `someConstInFunc` should have UPPER_SNAKE_CASE name, e.g. `SOME_CONST_IN_FUNC` - let WHY_VAR_IS_CAPS = 10; - // ^^^^^^^^^^^^^^^ Variable `WHY_VAR_IS_CAPS` should have snake_case name, e.g. `why_var_is_caps` - } -} -"#, - ); - } - - #[test] - fn no_diagnostic_for_enum_varinats() { - check_diagnostics( - r#" -enum Option { Some, None } - -fn main() { - match Option::None { - None => (), - Some => (), - } -} -"#, - ); - } - - #[test] - fn non_let_bind() { - check_diagnostics( - r#" -enum Option { Some, None } - -fn main() { - match Option::None { - SOME_VAR @ None => (), - // ^^^^^^^^ Variable `SOME_VAR` should have snake_case name, e.g. `some_var` - Some => (), - } -} -"#, - ); - } - - #[test] - fn allow_attributes() { - check_diagnostics( - r#" -#[allow(non_snake_case)] -fn NonSnakeCaseName(SOME_VAR: u8) -> u8{ - // cov_flags generated output from elsewhere in this file - extern "C" { - #[no_mangle] - static lower_case: u8; - } - - let OtherVar = SOME_VAR + 1; - OtherVar -} - -#[allow(nonstandard_style)] -mod CheckNonstandardStyle { - fn HiImABadFnName() {} -} - -#[allow(bad_style)] -mod CheckBadStyle { - fn HiImABadFnName() {} -} - -mod F { - #![allow(non_snake_case)] - fn CheckItWorksWithModAttr(BAD_NAME_HI: u8) {} -} - -#[allow(non_snake_case, non_camel_case_types)] -pub struct some_type { - SOME_FIELD: u8, - SomeField: u16, -} - -#[allow(non_upper_case_globals)] -pub const some_const: u8 = 10; - -#[allow(non_upper_case_globals)] -pub static SomeStatic: u8 = 10; - "#, - ); - } - - #[test] - fn allow_attributes_crate_attr() { - check_diagnostics( - r#" -#![allow(non_snake_case)] - -mod F { - fn CheckItWorksWithCrateAttr(BAD_NAME_HI: u8) {} -} - "#, - ); - } - - #[test] - #[ignore] - fn bug_trait_inside_fn() { - // FIXME: - // This is broken, and in fact, should not even be looked at by this - // lint in the first place. There's weird stuff going on in the - // collection phase. - // It's currently being brought in by: - // * validate_func on `a` recursing into modules - // * then it finds the trait and then the function while iterating - // through modules - // * then validate_func is called on Dirty - // * ... which then proceeds to look at some unknown module taking no - // attrs from either the impl or the fn a, and then finally to the root - // module - // - // It should find the attribute on the trait, but it *doesn't even see - // the trait* as far as I can tell. - - check_diagnostics( - r#" -trait T { fn a(); } -struct U {} -impl T for U { - fn a() { - // this comes out of bitflags, mostly - #[allow(non_snake_case)] - trait __BitFlags { - const HiImAlsoBad: u8 = 2; - #[inline] - fn Dirty(&self) -> bool { - false - } - } - - } -} - "#, - ); - } - - #[test] - #[ignore] - fn bug_traits_arent_checked() { - // FIXME: Traits and functions in traits aren't currently checked by - // r-a, even though rustc will complain about them. - check_diagnostics( - r#" -trait BAD_TRAIT { - // ^^^^^^^^^ Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait` - fn BAD_FUNCTION(); - // ^^^^^^^^^^^^ Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function` - fn BadFunction(); - // ^^^^^^^^^^^^ Function `BadFunction` should have snake_case name, e.g. `bad_function` -} - "#, - ); - } - - #[test] - fn ignores_extern_items() { - cov_mark::check!(extern_func_incorrect_case_ignored); - cov_mark::check!(extern_static_incorrect_case_ignored); - check_diagnostics( - r#" -extern { - fn NonSnakeCaseName(SOME_VAR: u8) -> u8; - pub static SomeStatic: u8 = 10; -} - "#, - ); - } - - #[test] - fn infinite_loop_inner_items() { - check_diagnostics( - r#" -fn qualify() { - mod foo { - use super::*; - } -} - "#, - ) - } - - #[test] // Issue #8809. - fn parenthesized_parameter() { - check_diagnostics(r#"fn f((O): _) {}"#) - } -} diff --git a/crates/hir_ty/src/diagnostics_sink.rs b/crates/hir_ty/src/diagnostics_sink.rs deleted file mode 100644 index 084fa8b06..000000000 --- a/crates/hir_ty/src/diagnostics_sink.rs +++ /dev/null @@ -1,109 +0,0 @@ -//! Semantic errors and warnings. -//! -//! The `Diagnostic` trait defines a trait object which can represent any -//! diagnostic. -//! -//! `DiagnosticSink` struct is used as an emitter for diagnostic. When creating -//! a `DiagnosticSink`, you supply a callback which can react to a `dyn -//! Diagnostic` or to any concrete diagnostic (downcasting is used internally). -//! -//! Because diagnostics store file offsets, it's a bad idea to store them -//! directly in salsa. For this reason, every hir subsytem defines it's own -//! strongly-typed closed set of diagnostics which use hir ids internally, are -//! stored in salsa and do *not* implement the `Diagnostic` trait. Instead, a -//! subsystem provides a separate, non-query-based API which can walk all stored -//! values and transform them into instances of `Diagnostic`. - -use std::{any::Any, fmt}; - -use hir_expand::InFile; -use syntax::SyntaxNodePtr; - -#[derive(Copy, Clone, Debug, PartialEq)] -pub struct DiagnosticCode(pub &'static str); - -impl DiagnosticCode { - pub fn as_str(&self) -> &str { - self.0 - } -} - -pub trait Diagnostic: Any + Send + Sync + fmt::Debug + 'static { - fn code(&self) -> DiagnosticCode; - fn message(&self) -> String; - /// Source element that triggered the diagnostics. - /// - /// Note that this should reflect "semantics", rather than specific span we - /// want to highlight. When rendering the diagnostics into an error message, - /// the IDE will fetch the `SyntaxNode` and will narrow the span - /// appropriately. - fn display_source(&self) -> InFile; - fn as_any(&self) -> &(dyn Any + Send + 'static); - fn is_experimental(&self) -> bool { - false - } -} - -pub struct DiagnosticSink<'a> { - callbacks: Vec Result<(), ()> + 'a>>, - filters: Vec bool + 'a>>, - default_callback: Box, -} - -impl<'a> DiagnosticSink<'a> { - pub fn push(&mut self, d: impl Diagnostic) { - let d: &dyn Diagnostic = &d; - self._push(d); - } - - fn _push(&mut self, d: &dyn Diagnostic) { - for filter in &mut self.filters { - if !filter(d) { - return; - } - } - for cb in &mut self.callbacks { - match cb(d) { - Ok(()) => return, - Err(()) => (), - } - } - (self.default_callback)(d) - } -} - -pub struct DiagnosticSinkBuilder<'a> { - callbacks: Vec Result<(), ()> + 'a>>, - filters: Vec bool + 'a>>, -} - -impl<'a> DiagnosticSinkBuilder<'a> { - pub fn new() -> Self { - Self { callbacks: Vec::new(), filters: Vec::new() } - } - - pub fn filter bool + 'a>(mut self, cb: F) -> Self { - self.filters.push(Box::new(cb)); - self - } - - pub fn on(mut self, mut cb: F) -> Self { - let cb = move |diag: &dyn Diagnostic| match diag.as_any().downcast_ref::() { - Some(d) => { - cb(d); - Ok(()) - } - None => Err(()), - }; - self.callbacks.push(Box::new(cb)); - self - } - - pub fn build(self, default_callback: F) -> DiagnosticSink<'a> { - DiagnosticSink { - callbacks: self.callbacks, - filters: self.filters, - default_callback: Box::new(default_callback), - } - } -} diff --git a/crates/hir_ty/src/lib.rs b/crates/hir_ty/src/lib.rs index 0c6b19653..128cae830 100644 --- a/crates/hir_ty/src/lib.rs +++ b/crates/hir_ty/src/lib.rs @@ -21,7 +21,6 @@ mod utils; mod walk; pub mod db; pub mod diagnostics; -pub mod diagnostics_sink; pub mod display; pub mod method_resolution; pub mod primitive; diff --git a/crates/ide/src/diagnostics.rs b/crates/ide/src/diagnostics.rs index dffb6fdc8..e34341631 100644 --- a/crates/ide/src/diagnostics.rs +++ b/crates/ide/src/diagnostics.rs @@ -1338,6 +1338,35 @@ fn main() { "#, ); } + + #[test] + fn import_extern_crate_clash_with_inner_item() { + // This is more of a resolver test, but doesn't really work with the hir_def testsuite. + + check_diagnostics( + r#" +//- /lib.rs crate:lib deps:jwt +mod permissions; + +use permissions::jwt; + +fn f() { + fn inner() {} + jwt::Claims {}; // should resolve to the local one with 0 fields, and not get a diagnostic +} + +//- /permissions.rs +pub mod jwt { + pub struct Claims {} +} + +//- /jwt/lib.rs crate:jwt +pub struct Claims { + field: u8, +} + "#, + ); + } } #[cfg(test)] @@ -2245,3 +2274,328 @@ fn main() { } } } + +#[cfg(test)] +mod decl_check_tests { + use crate::diagnostics::tests::check_diagnostics; + + #[test] + fn incorrect_function_name() { + check_diagnostics( + r#" +fn NonSnakeCaseName() {} +// ^^^^^^^^^^^^^^^^ Function `NonSnakeCaseName` should have snake_case name, e.g. `non_snake_case_name` +"#, + ); + } + + #[test] + fn incorrect_function_params() { + check_diagnostics( + r#" +fn foo(SomeParam: u8) {} + // ^^^^^^^^^ Parameter `SomeParam` should have snake_case name, e.g. `some_param` + +fn foo2(ok_param: &str, CAPS_PARAM: u8) {} + // ^^^^^^^^^^ Parameter `CAPS_PARAM` should have snake_case name, e.g. `caps_param` +"#, + ); + } + + #[test] + fn incorrect_variable_names() { + check_diagnostics( + r#" +fn foo() { + let SOME_VALUE = 10; + // ^^^^^^^^^^ Variable `SOME_VALUE` should have snake_case name, e.g. `some_value` + let AnotherValue = 20; + // ^^^^^^^^^^^^ Variable `AnotherValue` should have snake_case name, e.g. `another_value` +} +"#, + ); + } + + #[test] + fn incorrect_struct_names() { + check_diagnostics( + r#" +struct non_camel_case_name {} + // ^^^^^^^^^^^^^^^^^^^ Structure `non_camel_case_name` should have CamelCase name, e.g. `NonCamelCaseName` + +struct SCREAMING_CASE {} + // ^^^^^^^^^^^^^^ Structure `SCREAMING_CASE` should have CamelCase name, e.g. `ScreamingCase` +"#, + ); + } + + #[test] + fn no_diagnostic_for_camel_cased_acronyms_in_struct_name() { + check_diagnostics( + r#" +struct AABB {} +"#, + ); + } + + #[test] + fn incorrect_struct_field() { + check_diagnostics( + r#" +struct SomeStruct { SomeField: u8 } + // ^^^^^^^^^ Field `SomeField` should have snake_case name, e.g. `some_field` +"#, + ); + } + + #[test] + fn incorrect_enum_names() { + check_diagnostics( + r#" +enum some_enum { Val(u8) } + // ^^^^^^^^^ Enum `some_enum` should have CamelCase name, e.g. `SomeEnum` + +enum SOME_ENUM {} + // ^^^^^^^^^ Enum `SOME_ENUM` should have CamelCase name, e.g. `SomeEnum` +"#, + ); + } + + #[test] + fn no_diagnostic_for_camel_cased_acronyms_in_enum_name() { + check_diagnostics( + r#" +enum AABB {} +"#, + ); + } + + #[test] + fn incorrect_enum_variant_name() { + check_diagnostics( + r#" +enum SomeEnum { SOME_VARIANT(u8) } + // ^^^^^^^^^^^^ Variant `SOME_VARIANT` should have CamelCase name, e.g. `SomeVariant` +"#, + ); + } + + #[test] + fn incorrect_const_name() { + check_diagnostics( + r#" +const some_weird_const: u8 = 10; + // ^^^^^^^^^^^^^^^^ Constant `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST` +"#, + ); + } + + #[test] + fn incorrect_static_name() { + check_diagnostics( + r#" +static some_weird_const: u8 = 10; + // ^^^^^^^^^^^^^^^^ Static variable `some_weird_const` should have UPPER_SNAKE_CASE name, e.g. `SOME_WEIRD_CONST` +"#, + ); + } + + #[test] + fn fn_inside_impl_struct() { + check_diagnostics( + r#" +struct someStruct; + // ^^^^^^^^^^ Structure `someStruct` should have CamelCase name, e.g. `SomeStruct` + +impl someStruct { + fn SomeFunc(&self) { + // ^^^^^^^^ Function `SomeFunc` should have snake_case name, e.g. `some_func` + let WHY_VAR_IS_CAPS = 10; + // ^^^^^^^^^^^^^^^ Variable `WHY_VAR_IS_CAPS` should have snake_case name, e.g. `why_var_is_caps` + } +} +"#, + ); + } + + #[test] + fn no_diagnostic_for_enum_varinats() { + check_diagnostics( + r#" +enum Option { Some, None } + +fn main() { + match Option::None { + None => (), + Some => (), + } +} +"#, + ); + } + + #[test] + fn non_let_bind() { + check_diagnostics( + r#" +enum Option { Some, None } + +fn main() { + match Option::None { + SOME_VAR @ None => (), + // ^^^^^^^^ Variable `SOME_VAR` should have snake_case name, e.g. `some_var` + Some => (), + } +} +"#, + ); + } + + #[test] + fn allow_attributes() { + check_diagnostics( + r#" +#[allow(non_snake_case)] +fn NonSnakeCaseName(SOME_VAR: u8) -> u8{ + // cov_flags generated output from elsewhere in this file + extern "C" { + #[no_mangle] + static lower_case: u8; + } + + let OtherVar = SOME_VAR + 1; + OtherVar +} + +#[allow(nonstandard_style)] +mod CheckNonstandardStyle { + fn HiImABadFnName() {} +} + +#[allow(bad_style)] +mod CheckBadStyle { + fn HiImABadFnName() {} +} + +mod F { + #![allow(non_snake_case)] + fn CheckItWorksWithModAttr(BAD_NAME_HI: u8) {} +} + +#[allow(non_snake_case, non_camel_case_types)] +pub struct some_type { + SOME_FIELD: u8, + SomeField: u16, +} + +#[allow(non_upper_case_globals)] +pub const some_const: u8 = 10; + +#[allow(non_upper_case_globals)] +pub static SomeStatic: u8 = 10; + "#, + ); + } + + #[test] + fn allow_attributes_crate_attr() { + check_diagnostics( + r#" +#![allow(non_snake_case)] + +mod F { + fn CheckItWorksWithCrateAttr(BAD_NAME_HI: u8) {} +} + "#, + ); + } + + #[test] + #[ignore] + fn bug_trait_inside_fn() { + // FIXME: + // This is broken, and in fact, should not even be looked at by this + // lint in the first place. There's weird stuff going on in the + // collection phase. + // It's currently being brought in by: + // * validate_func on `a` recursing into modules + // * then it finds the trait and then the function while iterating + // through modules + // * then validate_func is called on Dirty + // * ... which then proceeds to look at some unknown module taking no + // attrs from either the impl or the fn a, and then finally to the root + // module + // + // It should find the attribute on the trait, but it *doesn't even see + // the trait* as far as I can tell. + + check_diagnostics( + r#" +trait T { fn a(); } +struct U {} +impl T for U { + fn a() { + // this comes out of bitflags, mostly + #[allow(non_snake_case)] + trait __BitFlags { + const HiImAlsoBad: u8 = 2; + #[inline] + fn Dirty(&self) -> bool { + false + } + } + + } +} + "#, + ); + } + + #[test] + #[ignore] + fn bug_traits_arent_checked() { + // FIXME: Traits and functions in traits aren't currently checked by + // r-a, even though rustc will complain about them. + check_diagnostics( + r#" +trait BAD_TRAIT { + // ^^^^^^^^^ Trait `BAD_TRAIT` should have CamelCase name, e.g. `BadTrait` + fn BAD_FUNCTION(); + // ^^^^^^^^^^^^ Function `BAD_FUNCTION` should have snake_case name, e.g. `bad_function` + fn BadFunction(); + // ^^^^^^^^^^^^ Function `BadFunction` should have snake_case name, e.g. `bad_function` +} + "#, + ); + } + + #[test] + fn ignores_extern_items() { + check_diagnostics( + r#" +extern { + fn NonSnakeCaseName(SOME_VAR: u8) -> u8; + pub static SomeStatic: u8 = 10; +} + "#, + ); + } + + #[test] + fn infinite_loop_inner_items() { + check_diagnostics( + r#" +fn qualify() { + mod foo { + use super::*; + } +} + "#, + ) + } + + #[test] // Issue #8809. + fn parenthesized_parameter() { + check_diagnostics(r#"fn f((O): _) {}"#) + } +} -- cgit v1.2.3