From 12b4d4f60a344b36a8d3cca46743db801e2c5d46 Mon Sep 17 00:00:00 2001 From: Emanuele Stoppa Date: Wed, 26 Jun 2024 16:41:51 +0100 Subject: [PATCH] feat: enable linting for graphql --- Cargo.lock | 1 + crates/biome_analyze/src/diagnostics.rs | 57 +++ crates/biome_analyze/src/lib.rs | 3 +- crates/biome_js_analyze/src/lib.rs | 59 +-- crates/biome_service/Cargo.toml | 1 + crates/biome_service/src/diagnostics.rs | 2 +- crates/biome_service/src/file_handlers/css.rs | 8 +- .../src/file_handlers/graphql.rs | 338 +++++++++++++++++- .../src/file_handlers/javascript.rs | 7 +- 9 files changed, 395 insertions(+), 81 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 00741863c263..48137f53bb75 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -956,6 +956,7 @@ dependencies = [ "biome_flags", "biome_formatter", "biome_fs", + "biome_graphql_analyze", "biome_graphql_parser", "biome_graphql_syntax", "biome_grit_patterns", diff --git a/crates/biome_analyze/src/diagnostics.rs b/crates/biome_analyze/src/diagnostics.rs index efec6d670faf..e2b48c6deced 100644 --- a/crates/biome_analyze/src/diagnostics.rs +++ b/crates/biome_analyze/src/diagnostics.rs @@ -4,6 +4,8 @@ use biome_diagnostics::{ DiagnosticTags, Error, Location, Severity, Visit, }; use biome_rowan::TextRange; +use serde::{Deserialize, Serialize}; +use std::borrow::Cow; use std::fmt::{Debug, Display, Formatter}; use crate::rule::RuleDiagnostic; @@ -171,3 +173,58 @@ impl SuppressionDiagnostic { self } } + +/// Series of errors encountered when running rules on a file +#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +pub enum RuleError { + /// The rule with the specified name replaced the root of the file with a node that is not a valid root for that language. + ReplacedRootWithNonRootError { + rule_name: Option<(Cow<'static, str>, Cow<'static, str>)>, + }, +} + +impl Diagnostic for RuleError {} + +impl std::fmt::Display for RuleError { + fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { + match self { + RuleError::ReplacedRootWithNonRootError { + rule_name: Some((group, rule)), + } => { + std::write!( + fmt, + "the rule '{group}/{rule}' replaced the root of the file with a non-root node." + ) + } + RuleError::ReplacedRootWithNonRootError { rule_name: None } => { + std::write!( + fmt, + "a code action replaced the root of the file with a non-root node." + ) + } + } + } +} + +impl biome_console::fmt::Display for RuleError { + fn fmt(&self, fmt: &mut biome_console::fmt::Formatter) -> std::io::Result<()> { + match self { + RuleError::ReplacedRootWithNonRootError { + rule_name: Some((group, rule)), + } => { + std::write!( + fmt, + "the rule '{group}/{rule}' replaced the root of the file with a non-root node." + ) + } + RuleError::ReplacedRootWithNonRootError { rule_name: None } => { + std::write!( + fmt, + "a code action replaced the root of the file with a non-root node." + ) + } + } + } +} + +impl std::error::Error for RuleError {} diff --git a/crates/biome_analyze/src/lib.rs b/crates/biome_analyze/src/lib.rs index 13f40594c378..22ad5e3cd8ad 100644 --- a/crates/biome_analyze/src/lib.rs +++ b/crates/biome_analyze/src/lib.rs @@ -27,8 +27,7 @@ pub use crate::categories::{ ActionCategory, RefactorKind, RuleCategories, RuleCategoriesBuilder, RuleCategory, SourceActionKind, }; -pub use crate::diagnostics::AnalyzerDiagnostic; -pub use crate::diagnostics::SuppressionDiagnostic; +pub use crate::diagnostics::{AnalyzerDiagnostic, RuleError, SuppressionDiagnostic}; pub use crate::matcher::{InspectMatcher, MatchQueryParams, QueryMatcher, RuleKey, SignalEntry}; pub use crate::options::{AnalyzerConfiguration, AnalyzerOptions, AnalyzerRules}; pub use crate::query::{AddVisitor, QueryKey, QueryMatch, Queryable}; diff --git a/crates/biome_js_analyze/src/lib.rs b/crates/biome_js_analyze/src/lib.rs index 2baff312c935..4cf20c80ab95 100644 --- a/crates/biome_js_analyze/src/lib.rs +++ b/crates/biome_js_analyze/src/lib.rs @@ -7,13 +7,11 @@ use biome_analyze::{ SuppressionKind, }; use biome_aria::{AriaProperties, AriaRoles}; -use biome_diagnostics::{category, Diagnostic, Error as DiagnosticError}; +use biome_diagnostics::{category, Error as DiagnosticError}; use biome_js_syntax::{JsFileSource, JsLanguage}; use biome_project::PackageJson; use biome_suppression::{parse_suppression_comment, SuppressionDiagnostic}; -use serde::{Deserialize, Serialize}; use std::sync::Arc; -use std::{borrow::Cow, error::Error}; mod assists; mod ast_utils; @@ -172,61 +170,6 @@ where ) } -/// Series of errors encountered when running rules on a file -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] -pub enum RuleError { - /// The rule with the specified name replaced the root of the file with a node that is not a valid root for that language. - ReplacedRootWithNonRootError { - rule_name: Option<(Cow<'static, str>, Cow<'static, str>)>, - }, -} - -impl Diagnostic for RuleError {} - -impl std::fmt::Display for RuleError { - fn fmt(&self, fmt: &mut std::fmt::Formatter) -> std::fmt::Result { - match self { - RuleError::ReplacedRootWithNonRootError { - rule_name: Some((group, rule)), - } => { - std::write!( - fmt, - "the rule '{group}/{rule}' replaced the root of the file with a non-root node." - ) - } - RuleError::ReplacedRootWithNonRootError { rule_name: None } => { - std::write!( - fmt, - "a code action replaced the root of the file with a non-root node." - ) - } - } - } -} - -impl biome_console::fmt::Display for RuleError { - fn fmt(&self, fmt: &mut biome_console::fmt::Formatter) -> std::io::Result<()> { - match self { - RuleError::ReplacedRootWithNonRootError { - rule_name: Some((group, rule)), - } => { - std::write!( - fmt, - "the rule '{group}/{rule}' replaced the root of the file with a non-root node." - ) - } - RuleError::ReplacedRootWithNonRootError { rule_name: None } => { - std::write!( - fmt, - "a code action replaced the root of the file with a non-root node." - ) - } - } - } -} - -impl Error for RuleError {} - #[cfg(test)] mod tests { use biome_analyze::options::RuleOptions; diff --git a/crates/biome_service/Cargo.toml b/crates/biome_service/Cargo.toml index 0135841a35c5..a757499bc774 100644 --- a/crates/biome_service/Cargo.toml +++ b/crates/biome_service/Cargo.toml @@ -27,6 +27,7 @@ biome_diagnostics = { workspace = true } biome_flags = { workspace = true } biome_formatter = { workspace = true, features = ["serde"] } biome_fs = { workspace = true, features = ["serde"] } +biome_graphql_analyze = { workspace = true } biome_graphql_parser = { workspace = true } biome_graphql_syntax = { workspace = true } biome_grit_patterns = { workspace = true } diff --git a/crates/biome_service/src/diagnostics.rs b/crates/biome_service/src/diagnostics.rs index 7247cf9ecdbe..86d4df5d2aff 100644 --- a/crates/biome_service/src/diagnostics.rs +++ b/crates/biome_service/src/diagnostics.rs @@ -1,4 +1,5 @@ use crate::workspace::DocumentFileSource; +use biome_analyze::RuleError; use biome_configuration::diagnostics::{ConfigurationDiagnostic, EditorConfigDiagnostic}; use biome_configuration::{BiomeDiagnostic, CantLoadExtendFile}; use biome_console::fmt::Bytes; @@ -10,7 +11,6 @@ use biome_formatter::{FormatError, PrintError}; use biome_fs::{BiomePath, FileSystemDiagnostic}; use biome_grit_patterns::CompileError; use biome_js_analyze::utils::rename::RenameError; -use biome_js_analyze::RuleError; use serde::{Deserialize, Serialize}; use std::error::Error; use std::ffi::OsStr; diff --git a/crates/biome_service/src/file_handlers/css.rs b/crates/biome_service/src/file_handlers/css.rs index be33cf500130..a10030bdfefe 100644 --- a/crates/biome_service/src/file_handlers/css.rs +++ b/crates/biome_service/src/file_handlers/css.rs @@ -19,7 +19,7 @@ use crate::WorkspaceError; use biome_analyze::options::PreferredQuote; use biome_analyze::{ AnalysisFilter, AnalyzerConfiguration, AnalyzerOptions, ControlFlow, Never, - RuleCategoriesBuilder, RuleCategory, + RuleCategoriesBuilder, RuleCategory, RuleError, }; use biome_css_analyze::analyze; use biome_css_formatter::context::CssFormatOptions; @@ -31,8 +31,6 @@ use biome_formatter::{ FormatError, IndentStyle, IndentWidth, LineEnding, LineWidth, Printed, QuoteStyle, }; use biome_fs::BiomePath; -use biome_js_analyze::RuleError; -use biome_js_syntax::AnyJsRoot; use biome_parser::AnyParse; use biome_rowan::{AstNode, NodeCache}; use biome_rowan::{TextRange, TextSize, TokenAtOffset}; @@ -476,7 +474,7 @@ pub(crate) fn code_actions(params: CodeActionsParams) -> PullActionsResult { language, settings, } = params; - debug_span!("Code actions JavaScript", range =? range, path =? path).in_scope(move || { + debug_span!("Code actions CSS", range =? range, path =? path).in_scope(move || { let tree = parse.tree(); trace_span!("Parsed file", tree =? tree).in_scope(move || { let rules = settings.as_rules(params.path.as_path()); @@ -539,7 +537,7 @@ pub(crate) fn fix_all(params: FixAllParams) -> Result GetSyntaxTreeRe } fn lint(params: LintParams) -> LintResults { - let diagnostics = params.parse.into_diagnostics(); - LintResults { - errors: diagnostics.len(), - diagnostics, - skipped_diagnostics: 0, + debug_span!("Linting GraphQL file", path =? params.path, language =? params.language).in_scope( + move || { + let workspace_settings = ¶ms.workspace; + let analyzer_options = workspace_settings + .analyzer_options::(params.path, ¶ms.language); + let tree = params.parse.tree(); + let mut diagnostics = params.parse.into_diagnostics(); + + let has_only_filter = !params.only.is_empty(); + let mut rules = None; + + let enabled_rules = if let Some(settings) = params.workspace.settings() { + // Compute final rules (taking `overrides` into account) + rules = settings.as_rules(params.path.as_path()); + + if has_only_filter { + params + .only + .into_iter() + .map(|selector| selector.into()) + .collect::>() + } else { + rules + .as_ref() + .map(|rules| rules.as_enabled_rules()) + .unwrap_or_default() + .into_iter() + .collect::>() + } + } else { + vec![] + }; + + let disabled_rules = params + .skip + .into_iter() + .map(|selector| selector.into()) + .collect::>(); + + let filter = AnalysisFilter { + categories: params.categories, + enabled_rules: Some(enabled_rules.as_slice()), + disabled_rules: &disabled_rules, + range: None, + }; + + // Do not report unused suppression comment diagnostics if: + // - it is a syntax-only analyzer pass, or + // - if a single rule is run. + let ignores_suppression_comment = + !filter.categories.contains(RuleCategory::Lint) || has_only_filter; + + let mut diagnostic_count = diagnostics.len() as u32; + let mut errors = diagnostics + .iter() + .filter(|diag| diag.severity() <= Severity::Error) + .count(); + + info!("Analyze file {}", params.path.display()); + let (_, analyze_diagnostics) = analyze(&tree, filter, &analyzer_options, |signal| { + if let Some(mut diagnostic) = signal.diagnostic() { + // Do not report unused suppression comment diagnostics if this is a syntax-only analyzer pass + if ignores_suppression_comment + && diagnostic.category() == Some(category!("suppressions/unused")) + { + return ControlFlow::::Continue(()); + } + + diagnostic_count += 1; + + // We do now check if the severity of the diagnostics should be changed. + // The configuration allows to change the severity of the diagnostics emitted by rules. + let severity = diagnostic + .category() + .filter(|category| category.name().starts_with("lint/")) + .map_or_else( + || diagnostic.severity(), + |category| { + rules + .as_ref() + .and_then(|rules| rules.get_severity_from_code(category)) + .unwrap_or(Severity::Warning) + }, + ); + + if severity >= Severity::Error { + errors += 1; + } + + if diagnostic_count <= params.max_diagnostics { + for action in signal.actions() { + if !action.is_suppression() { + diagnostic = diagnostic.add_code_suggestion(action.into()); + } + } + + let error = diagnostic.with_severity(severity); + + diagnostics.push(biome_diagnostics::serde::Diagnostic::new(error)); + } + } + + ControlFlow::::Continue(()) + }); + + diagnostics.extend( + analyze_diagnostics + .into_iter() + .map(biome_diagnostics::serde::Diagnostic::new) + .collect::>(), + ); + let skipped_diagnostics = diagnostic_count.saturating_sub(diagnostics.len() as u32); + + LintResults { + diagnostics, + errors, + skipped_diagnostics, + } + }, + ) +} + +#[tracing::instrument(level = "debug", skip(params))] +pub(crate) fn code_actions(params: CodeActionsParams) -> PullActionsResult { + let CodeActionsParams { + parse, + range, + workspace, + path, + manifest: _, + language, + settings, + } = params; + debug_span!("Code actions GraphQL", range =? range, path =? path).in_scope(move || { + let tree = parse.tree(); + trace_span!("Parsed file", tree =? tree).in_scope(move || { + let rules = settings.as_rules(params.path.as_path()); + let filter = rules + .as_ref() + .map(|rules| rules.as_enabled_rules()) + .unwrap_or_default() + .into_iter() + .collect::>(); + + let mut filter = AnalysisFilter::from_enabled_rules(filter.as_slice()); + + let mut categories = RuleCategoriesBuilder::default().with_syntax().with_lint(); + if settings.organize_imports.enabled { + categories = categories.with_action(); + } + filter.categories = categories.build(); + filter.range = Some(range); + + let analyzer_options = workspace.analyzer_options::(path, &language); + + let Some(_) = language.to_graphql_file_source() else { + error!("Could not determine the file source of the file"); + return PullActionsResult { actions: vec![] }; + }; + + trace!("GraphQL runs the analyzer"); + let mut actions = Vec::new(); + + analyze(&tree, filter, &analyzer_options, |signal| { + actions.extend(signal.actions().into_code_action_iter().map(|item| { + CodeAction { + category: item.category.clone(), + rule_name: item + .rule_name + .map(|(group, name)| (Cow::Borrowed(group), Cow::Borrowed(name))), + suggestion: item.suggestion, + } + })); + + ControlFlow::::Continue(()) + }); + + PullActionsResult { actions } + }) + }) +} + +/// If applies all the safe fixes to the given syntax tree. +pub(crate) fn fix_all(params: FixAllParams) -> Result { + let FixAllParams { + parse, + fix_file_mode, + workspace, + should_format: _, // we don't have a formatter yet + biome_path, + manifest: _, + document_file_source, + } = params; + + let settings = workspace.settings(); + let Some(settings) = settings else { + let tree: GraphqlRoot = parse.tree(); + + return Ok(FixFileResult { + actions: vec![], + errors: 0, + skipped_suggested_fixes: 0, + code: tree.syntax().to_string(), + }); + }; + + let mut tree: GraphqlRoot = parse.tree(); + let mut actions = Vec::new(); + + // Compute final rules (taking `overrides` into account) + let rules = settings.as_rules(params.biome_path.as_path()); + let rule_filter_list = rules + .as_ref() + .map(|rules| rules.as_enabled_rules()) + .unwrap_or_default() + .into_iter() + .collect::>(); + let filter = AnalysisFilter::from_enabled_rules(rule_filter_list.as_slice()); + + let mut skipped_suggested_fixes = 0; + let mut errors: u16 = 0; + let analyzer_options = + workspace.analyzer_options::(biome_path, &document_file_source); + loop { + let (action, _) = analyze(&tree, filter, &analyzer_options, |signal| { + let current_diagnostic = signal.diagnostic(); + + if let Some(diagnostic) = current_diagnostic.as_ref() { + if is_diagnostic_error(diagnostic, rules.as_deref()) { + errors += 1; + } + } + + for action in signal.actions() { + // suppression actions should not be part of the fixes (safe or suggested) + if action.is_suppression() { + continue; + } + + match fix_file_mode { + FixFileMode::SafeFixes => { + if action.applicability == Applicability::MaybeIncorrect { + skipped_suggested_fixes += 1; + } + if action.applicability == Applicability::Always { + errors = errors.saturating_sub(1); + return ControlFlow::Break(action); + } + } + FixFileMode::SafeAndUnsafeFixes => { + if matches!( + action.applicability, + Applicability::Always | Applicability::MaybeIncorrect + ) { + errors = errors.saturating_sub(1); + return ControlFlow::Break(action); + } + } + } + } + + ControlFlow::Continue(()) + }); + + match action { + Some(action) => { + if let (root, Some((range, _))) = + action.mutation.commit_with_text_range_and_edit(true) + { + tree = match GraphqlRoot::cast(root) { + Some(tree) => tree, + None => { + return Err(WorkspaceError::RuleError( + RuleError::ReplacedRootWithNonRootError { + rule_name: action.rule_name.map(|(group, rule)| { + (Cow::Borrowed(group), Cow::Borrowed(rule)) + }), + }, + )); + } + }; + actions.push(FixAction { + rule_name: action + .rule_name + .map(|(group, rule)| (Cow::Borrowed(group), Cow::Borrowed(rule))), + range, + }); + } + } + None => { + // let code = if should_format { + // format_node( + // workspace.format_options::(biome_path, &document_file_source), + // tree.syntax(), + // )? + // .print()? + // .into_code() + // } else { + let code = tree.syntax().to_string(); + // }; + return Ok(FixFileResult { + code, + skipped_suggested_fixes, + actions, + errors: errors.into(), + }); + } + } } } diff --git a/crates/biome_service/src/file_handlers/javascript.rs b/crates/biome_service/src/file_handlers/javascript.rs index febda3f262f6..93ac66f774f6 100644 --- a/crates/biome_service/src/file_handlers/javascript.rs +++ b/crates/biome_service/src/file_handlers/javascript.rs @@ -21,7 +21,8 @@ use crate::{ use biome_analyze::options::PreferredQuote; use biome_analyze::{ AnalysisFilter, AnalyzerConfiguration, AnalyzerOptions, ControlFlow, GroupCategory, Never, - QueryMatch, RegistryVisitor, RuleCategoriesBuilder, RuleCategory, RuleFilter, RuleGroup, + QueryMatch, RegistryVisitor, RuleCategoriesBuilder, RuleCategory, RuleError, RuleFilter, + RuleGroup, }; use biome_configuration::javascript::JsxRuntime; use biome_diagnostics::{category, Applicability, Diagnostic, DiagnosticExt, Severity}; @@ -31,9 +32,7 @@ use biome_formatter::{ }; use biome_fs::BiomePath; use biome_js_analyze::utils::rename::{RenameError, RenameSymbolExtensions}; -use biome_js_analyze::{ - analyze, analyze_with_inspect_matcher, visit_registry, ControlFlowGraph, RuleError, -}; +use biome_js_analyze::{analyze, analyze_with_inspect_matcher, visit_registry, ControlFlowGraph}; use biome_js_formatter::context::trailing_commas::TrailingCommas; use biome_js_formatter::context::{ ArrowParentheses, BracketSameLine, BracketSpacing, JsFormatOptions, QuoteProperties, Semicolons,