From d40b67e3cfd32ae4d82470e15016e522f7eb163f Mon Sep 17 00:00:00 2001 From: scarf Date: Fri, 20 Mar 2026 16:22:13 +0900 Subject: [PATCH] feat: add warning-aware import diagnostics Demote import prefix and unversioned import findings to warnings so workspace import maps are easier to adopt. Make their autofixes update deno.json(c) only when the lockfile can supply a concrete version. Assisted-by: openai/gpt-5.4 on opencode Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- Cargo.lock | 10 + Cargo.toml | 1 + examples/dlint/diagnostics.rs | 12 +- examples/dlint/main.rs | 63 ++- src/context.rs | 112 ++++- src/diagnostic.rs | 35 +- src/rules.rs | 1 + src/rules/import_config.rs | 648 +++++++++++++++++++++++++ src/rules/jsx_boolean_value.rs | 1 + src/rules/jsx_curly_braces.rs | 3 + src/rules/jsx_no_unescaped_entities.rs | 1 + src/rules/jsx_props_no_spread_multi.rs | 1 + src/rules/no_import_prefix.rs | 193 +++++++- src/rules/no_node_globals.rs | 1 + src/rules/no_process_global.rs | 1 + src/rules/no_unversioned_import.rs | 257 +++++++++- src/rules/no_window.rs | 1 + src/rules/no_window_prefix.rs | 1 + src/rules/verbatim_module_syntax.rs | 7 + src/test_util.rs | 104 +++- 20 files changed, 1389 insertions(+), 64 deletions(-) create mode 100644 src/rules/import_config.rs diff --git a/Cargo.lock b/Cargo.lock index 52f6c0d8..c7498583 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -348,6 +348,7 @@ dependencies = [ "env_logger", "globwalk", "if_chain", + "jsonc-parser", "log", "once_cell", "os_pipe", @@ -797,6 +798,15 @@ version = "1.0.15" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" +[[package]] +name = "jsonc-parser" +version = "0.31.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "45157f76d8fbe134e0c97403d69ac48989a1e0249a058b20f9f5eeff11bb09f0" +dependencies = [ + "serde", +] + [[package]] name = "libc" version = "0.2.175" diff --git a/Cargo.toml b/Cargo.toml index 10ad0cd3..93d1ad8e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -34,6 +34,7 @@ anyhow = "1.0.79" if_chain = "1.0.2" phf = { version = "0.11.2", features = ["macros"] } deno_semver = "0.9.0" +jsonc-parser = { version = "0.31.0", features = ["cst", "serde"] } [dev-dependencies] ansi_term = "0.12.1" diff --git a/examples/dlint/diagnostics.rs b/examples/dlint/diagnostics.rs index d010d83d..e895dd87 100644 --- a/examples/dlint/diagnostics.rs +++ b/examples/dlint/diagnostics.rs @@ -1,7 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use deno_ast::diagnostics::Diagnostic; -use deno_lint::diagnostic::LintDiagnostic; +use deno_lint::diagnostic::{LintDiagnostic, LintDiagnosticSeverity}; pub fn display_diagnostics( diagnostics: &[LintDiagnostic], @@ -21,10 +21,11 @@ fn print_compact(diagnostics: &[LintDiagnostic]) { let display_index = range.text_info.line_and_column_display(range.range.start); eprintln!( - "{}: line {}, col {}, Error - {} ({})", + "{}: line {}, col {}, {} - {} ({})", diagnostic.specifier, display_index.line_number, display_index.column_number, + severity_label(diagnostic.severity()), diagnostic.details.message, diagnostic.details.code ) @@ -46,3 +47,10 @@ fn print_pretty(diagnostics: &[LintDiagnostic]) { eprintln!("{}\n", diagnostic.display()); } } + +fn severity_label(severity: LintDiagnosticSeverity) -> &'static str { + match severity { + LintDiagnosticSeverity::Error => "Error", + LintDiagnosticSeverity::Warning => "Warning", + } +} diff --git a/examples/dlint/main.rs b/examples/dlint/main.rs index 86c7184f..63f52037 100644 --- a/examples/dlint/main.rs +++ b/examples/dlint/main.rs @@ -8,6 +8,7 @@ use core::panic; use deno_ast::diagnostics::Diagnostic; use deno_ast::MediaType; use deno_ast::ModuleSpecifier; +use deno_lint::diagnostic::LintDiagnosticSeverity; use deno_lint::linter::LintConfig; use deno_lint::linter::LintFileOptions; use deno_lint::linter::Linter; @@ -86,7 +87,8 @@ fn run_linter( paths.extend(config.get_files()?); } - let error_counts = Arc::new(AtomicUsize::new(0)); + let error_count = Arc::new(AtomicUsize::new(0)); + let warning_count = Arc::new(AtomicUsize::new(0)); let all_rules = get_all_rules(); let all_rule_codes = all_rules @@ -138,7 +140,18 @@ fn run_linter( external_linter: None, })?; - let mut number_of_errors = diagnostics.len(); + let mut number_of_errors = diagnostics + .iter() + .filter(|diagnostic| { + diagnostic.severity() == LintDiagnosticSeverity::Error + }) + .count(); + let number_of_warnings = diagnostics + .iter() + .filter(|diagnostic| { + diagnostic.severity() == LintDiagnosticSeverity::Warning + }) + .count(); if !parsed_source.diagnostics().is_empty() { number_of_errors += parsed_source.diagnostics().to_vec().len(); parsed_source.diagnostics().to_vec().iter().for_each( @@ -148,7 +161,8 @@ fn run_linter( ); } - error_counts.fetch_add(number_of_errors, Ordering::Relaxed); + error_count.fetch_add(number_of_errors, Ordering::Relaxed); + warning_count.fetch_add(number_of_warnings, Ordering::Relaxed); let mut lock = file_diagnostics.lock().unwrap(); @@ -161,19 +175,50 @@ fn run_linter( diagnostics::display_diagnostics(d, format); } - let err_count = error_counts.load(Ordering::Relaxed); + let err_count = error_count.load(Ordering::Relaxed); + let warn_count = warning_count.load(Ordering::Relaxed); + if err_count > 0 || warn_count > 0 { + eprintln!("{}", format_problem_counts(err_count, warn_count)); + } if err_count > 0 { - eprintln!( - "Found {} problem{}", - err_count, - if err_count == 1 { "" } else { "s" } - ); std::process::exit(1); } Ok(()) } +fn format_problem_counts(errors: usize, warnings: usize) -> String { + if warnings == 0 { + let total = errors; + return format!( + "Found {} problem{}", + total, + if total == 1 { "" } else { "s" } + ); + } + + if errors == 0 { + return format!( + "Found {} warning{}", + warnings, + if warnings == 1 { "" } else { "s" } + ); + } + + let mut parts = Vec::new(); + parts.push(format!( + "{} error{}", + errors, + if errors == 1 { "" } else { "s" } + )); + parts.push(format!( + "{} warning{}", + warnings, + if warnings == 1 { "" } else { "s" } + )); + format!("Found {}", parts.join(", ")) +} + fn main() -> Result<(), AnyError> { env_logger::init(); diff --git a/src/context.rs b/src/context.rs index 5c0865a4..7fddfc56 100644 --- a/src/context.rs +++ b/src/context.rs @@ -2,8 +2,8 @@ use crate::control_flow::ControlFlow; use crate::diagnostic::{ - LintDiagnostic, LintDiagnosticDetails, LintDiagnosticRange, LintDocsUrl, - LintFix, + LintDiagnostic, LintDiagnosticDetails, LintDiagnosticRange, + LintDiagnosticSeverity, LintDocsUrl, LintFix, }; use crate::ignore_directives::{ parse_line_ignore_directives, CodeStatus, FileIgnoreDirective, @@ -300,6 +300,7 @@ impl<'a> Context<'a> { format!("Ignore for code \"{}\" was not used.", unused_code), None, Vec::new(), + LintDiagnosticSeverity::Error, ), ); diagnostics.push(d); @@ -321,6 +322,7 @@ impl<'a> Context<'a> { format!("Ignore for code \"{}\" was not used.", unused_code), None, Vec::new(), + LintDiagnosticSeverity::Error, ), ); diagnostics.push(d); @@ -353,6 +355,7 @@ impl<'a> Context<'a> { format!("Unknown rule for code \"{}\"", unknown_rule_code), None, Vec::new(), + LintDiagnosticSeverity::Error, ), ); diagnostics.push(d); @@ -372,6 +375,7 @@ impl<'a> Context<'a> { format!("Unknown rule for code \"{}\"", unknown_rule_code), None, Vec::new(), + LintDiagnosticSeverity::Error, ), ); diagnostics.push(d); @@ -402,14 +406,13 @@ impl<'a> Context<'a> { code: impl ToString, message: impl ToString, ) { - self.add_diagnostic_details( - Some(self.create_diagnostic_range(range)), - self.create_diagnostic_details( - code, - message.to_string(), - None, - Vec::new(), - ), + self.add_diagnostic_with_hint_and_fixes( + range, + code, + message, + None, + Vec::new(), + LintDiagnosticSeverity::Error, ); } @@ -420,14 +423,13 @@ impl<'a> Context<'a> { message: impl ToString, hint: impl ToString, ) { - self.add_diagnostic_details( - Some(self.create_diagnostic_range(range)), - self.create_diagnostic_details( - code, - message, - Some(hint.to_string()), - Vec::new(), - ), + self.add_diagnostic_with_hint_and_fixes( + range, + code, + message, + Some(hint.to_string()), + Vec::new(), + LintDiagnosticSeverity::Error, ); } @@ -438,10 +440,80 @@ impl<'a> Context<'a> { message: impl ToString, hint: Option, fixes: Vec, + ) { + self.add_diagnostic_with_hint_and_fixes( + range, + code, + message, + hint, + fixes, + LintDiagnosticSeverity::Error, + ); + } + + pub fn add_warning( + &mut self, + range: SourceRange, + code: impl ToString, + message: impl ToString, + ) { + self.add_diagnostic_with_hint_and_fixes( + range, + code, + message, + None, + Vec::new(), + LintDiagnosticSeverity::Warning, + ); + } + + pub fn add_warning_with_hint( + &mut self, + range: SourceRange, + code: impl ToString, + message: impl ToString, + hint: impl ToString, + ) { + self.add_diagnostic_with_hint_and_fixes( + range, + code, + message, + Some(hint.to_string()), + Vec::new(), + LintDiagnosticSeverity::Warning, + ); + } + + pub fn add_warning_with_fixes( + &mut self, + range: SourceRange, + code: impl ToString, + message: impl ToString, + hint: Option, + fixes: Vec, + ) { + self.add_diagnostic_with_hint_and_fixes( + range, + code, + message, + hint, + fixes, + LintDiagnosticSeverity::Warning, + ); + } + + fn add_diagnostic_with_hint_and_fixes( + &mut self, + range: SourceRange, + code: impl ToString, + message: impl ToString, + hint: Option, + fixes: Vec, + severity: LintDiagnosticSeverity, ) { self.add_diagnostic_details( Some(self.create_diagnostic_range(range)), - self.create_diagnostic_details(code, message, hint, fixes), + self.create_diagnostic_details(code, message, hint, fixes, severity), ); } @@ -481,8 +553,10 @@ impl<'a> Context<'a> { message: impl ToString, maybe_hint: Option, fixes: Vec, + severity: LintDiagnosticSeverity, ) -> LintDiagnosticDetails { LintDiagnosticDetails { + severity, message: message.to_string(), code: code.to_string(), hint: maybe_hint, diff --git a/src/diagnostic.rs b/src/diagnostic.rs index 00600bd3..025d1761 100644 --- a/src/diagnostic.rs +++ b/src/diagnostic.rs @@ -14,8 +14,32 @@ use deno_ast::ModuleSpecifier; use deno_ast::SourceRange; use deno_ast::SourceTextInfo; +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum LintDiagnosticSeverity { + #[default] + Error, + Warning, +} + +impl LintDiagnosticSeverity { + fn diagnostic_level(self) -> DiagnosticLevel { + match self { + Self::Error => DiagnosticLevel::Error, + Self::Warning => DiagnosticLevel::Warning, + } + } + + fn highlight_style(self) -> DiagnosticSnippetHighlightStyle { + match self { + Self::Error => DiagnosticSnippetHighlightStyle::Error, + Self::Warning => DiagnosticSnippetHighlightStyle::Warning, + } + } +} + #[derive(Debug, Clone)] pub struct LintFixChange { + pub specifier: Option, pub new_text: Cow<'static, str>, pub range: SourceRange, } @@ -44,6 +68,7 @@ pub enum LintDocsUrl { #[derive(Clone)] pub struct LintDiagnosticDetails { + pub severity: LintDiagnosticSeverity, pub message: String, pub code: String, pub hint: Option, @@ -72,9 +97,15 @@ pub struct LintDiagnostic { pub details: LintDiagnosticDetails, } +impl LintDiagnostic { + pub fn severity(&self) -> LintDiagnosticSeverity { + self.details.severity + } +} + impl Diagnostic for LintDiagnostic { fn level(&self) -> DiagnosticLevel { - DiagnosticLevel::Error + self.details.severity.diagnostic_level() } fn code(&self) -> Cow<'_, str> { @@ -107,7 +138,7 @@ impl Diagnostic for LintDiagnostic { start: DiagnosticSourcePos::SourcePos(range.range.start), end: DiagnosticSourcePos::SourcePos(range.range.end), }, - style: DiagnosticSnippetHighlightStyle::Error, + style: self.details.severity.highlight_style(), description: range.description.as_deref().map(Cow::Borrowed), }], }) diff --git a/src/rules.rs b/src/rules.rs index b5f9fcfc..37b9435e 100644 --- a/src/rules.rs +++ b/src/rules.rs @@ -26,6 +26,7 @@ pub mod fresh_handler_export; pub mod fresh_server_event_handlers; pub mod getter_return; pub mod guard_for_in; +mod import_config; pub mod jsx_boolean_value; pub mod jsx_button_has_type; pub mod jsx_curly_braces; diff --git a/src/rules/import_config.rs b/src/rules/import_config.rs new file mode 100644 index 00000000..cbe39f2d --- /dev/null +++ b/src/rules/import_config.rs @@ -0,0 +1,648 @@ +use crate::context::Context; +use crate::diagnostic::{LintFix, LintFixChange}; +use deno_ast::{ModuleSpecifier, SourceRange, SourceTextInfo}; +use deno_semver::jsr::JsrPackageReqReference; +use deno_semver::npm::NpmPackageReqReference; +use jsonc_parser::cst::{CstInputValue, CstLeafNode, CstNode, CstRootNode}; +use jsonc_parser::ParseOptions; +use serde_json::Value; +use std::fs; +use std::path::{Path, PathBuf}; + +#[derive(Clone, Debug)] +struct ImportMapEntry { + key: String, + value: String, +} + +#[derive(Clone, Debug)] +struct DenoConfigFile { + specifier: ModuleSpecifier, + text: String, + imports: Vec, + lockfile: Option, +} + +#[derive(Clone, Debug, Default)] +struct LockfileData { + resolved_packages: Vec, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct ResolvedPackageVersion { + kind: PackageKind, + name: String, + version: String, +} + +pub fn fix_with_deno_config_import( + ctx: &Context, + range: SourceRange, + specifier_text: &str, + description: &'static str, +) -> Option { + let package_specifier = PackageSpecifier::parse(specifier_text); + if package_specifier + .as_ref() + .is_some_and(PackageSpecifier::is_unversioned) + { + return None; + } + fix_with_deno_config( + ctx, + range, + specifier_text, + description, + package_specifier, + ) +} + +pub fn fix_with_deno_config_package( + ctx: &Context, + range: SourceRange, + specifier_text: &str, + description: &'static str, +) -> Option { + fix_with_deno_config( + ctx, + range, + specifier_text, + description, + Some(PackageSpecifier::parse(specifier_text)?), + ) +} + +fn fix_with_deno_config( + ctx: &Context, + range: SourceRange, + specifier_text: &str, + description: &'static str, + maybe_package_specifier: Option, +) -> Option { + let config = DenoConfigFile::for_source_file(ctx.specifier())?; + + if let Some(replacement) = + rewrite_with_import_map_target(specifier_text, &config.imports) + { + return Some(source_only_fix(range, replacement, description)); + } + + if let Some(replacement) = + rewrite_with_package_match(specifier_text, &config.imports) + { + return Some(source_only_fix(range, replacement, description)); + } + + let package_specifier = maybe_package_specifier?; + let suggested_import = + package_specifier.suggested_import(config.lockfile.as_ref()?)?; + + if config + .imports + .iter() + .any(|entry| entry.key == suggested_import.key) + { + return None; + } + + let config_text = add_import_to_config(&config.text, &suggested_import)?; + + Some(LintFix { + description: description.into(), + changes: vec![ + source_change(range, suggested_import.replacement.clone()), + config_change(&config.specifier, &config.text, config_text), + ], + }) +} + +fn source_only_fix( + range: SourceRange, + replacement: String, + description: &'static str, +) -> LintFix { + LintFix { + description: description.into(), + changes: vec![source_change(range, replacement)], + } +} + +fn source_change(range: SourceRange, replacement: String) -> LintFixChange { + LintFixChange { + specifier: None, + new_text: format!("\"{}\"", replacement).into(), + range, + } +} + +fn config_change( + specifier: &ModuleSpecifier, + old_text: &str, + new_text: String, +) -> LintFixChange { + let text_info = SourceTextInfo::new(old_text.into()); + let range = text_info.range(); + LintFixChange { + specifier: Some(specifier.clone()), + new_text: new_text.into(), + range: SourceRange::new(range.start.as_source_pos(), range.end), + } +} + +impl DenoConfigFile { + fn for_source_file(source_specifier: &ModuleSpecifier) -> Option { + let source_path = source_specifier.to_file_path().ok()?; + let config_path = find_nearest_deno_config(&source_path)?; + let text = fs::read_to_string(&config_path).ok()?; + let specifier = ModuleSpecifier::from_file_path(&config_path).ok()?; + let config_value = + jsonc_parser::parse_to_serde_value::(&text, &Default::default()) + .ok() + .flatten(); + Some(Self { + imports: load_imports(&text), + lockfile: resolve_lockfile(&config_path, config_value.as_ref()), + specifier, + text, + }) + } +} + +fn find_nearest_deno_config(file_path: &Path) -> Option { + for dir in file_path.parent()?.ancestors() { + let deno_json = dir.join("deno.json"); + if deno_json.exists() { + return Some(deno_json); + } + + let deno_jsonc = dir.join("deno.jsonc"); + if deno_jsonc.exists() { + return Some(deno_jsonc); + } + } + + None +} + +fn load_imports(text: &str) -> Vec { + let Ok(Some(value)) = + jsonc_parser::parse_to_serde_value::(text, &Default::default()) + else { + return Vec::new(); + }; + let Some(imports) = value.get("imports").and_then(|value| value.as_object()) + else { + return Vec::new(); + }; + imports + .iter() + .filter_map(|(key, value)| { + value.as_str().map(|value| ImportMapEntry { + key: key.to_string(), + value: value.to_string(), + }) + }) + .collect() +} + +fn resolve_lockfile( + config_path: &Path, + config_value: Option<&Value>, +) -> Option { + let lockfile_path = lockfile_path(config_path, config_value)?; + let text = fs::read_to_string(lockfile_path).ok()?; + parse_lockfile(&text) +} + +fn lockfile_path( + config_path: &Path, + config_value: Option<&Value>, +) -> Option { + let config_dir = config_path.parent()?; + let lock_value = config_value.and_then(|value| value.get("lock")); + + match lock_value { + Some(Value::Bool(false)) => None, + Some(Value::String(path)) => Some(config_dir.join(path)), + Some(Value::Object(object)) => match object.get("path") { + Some(Value::String(path)) => Some(config_dir.join(path)), + Some(_) => None, + None => Some(config_dir.join("deno.lock")), + }, + Some(_) | None => Some(config_dir.join("deno.lock")), + } +} + +fn parse_lockfile(text: &str) -> Option { + let value = serde_json::from_str::(text).ok()?; + let root = value.as_object()?; + + let mut resolved_packages = Vec::new(); + + if let Some(specifiers) = + root.get("specifiers").and_then(|value| value.as_object()) + { + for value in specifiers.values() { + let Some(value) = value.as_str() else { + continue; + }; + if let Some(package) = resolved_package_from_lockfile_value(value) { + resolved_packages.push(package); + } + } + } + + if let Some(jsr) = root.get("jsr").and_then(|value| value.as_object()) { + for key in jsr.keys() { + if let Some(package) = + resolved_package_from_lockfile_value(&format!("jsr:{key}")) + { + resolved_packages.push(package); + } + } + } + + if let Some(npm) = root.get("npm").and_then(|value| value.as_object()) { + for key in npm.keys() { + if let Some(package) = + resolved_package_from_lockfile_value(&format!("npm:{key}")) + { + resolved_packages.push(package); + } + } + } + + Some(LockfileData { resolved_packages }) +} + +fn resolved_package_from_lockfile_value( + value: &str, +) -> Option { + let normalized = if let Some(value) = value.strip_prefix("npm:") { + format!( + "npm:{}", + value + .split_once('_') + .map(|(value, _)| value) + .unwrap_or(value) + ) + } else { + value.to_string() + }; + let package = PackageSpecifier::parse(&normalized)?; + Some(ResolvedPackageVersion { + kind: package.kind, + name: package.name, + version: package.version_req?, + }) +} + +fn add_import_to_config( + text: &str, + import: &SuggestedImport, +) -> Option { + let root = CstRootNode::parse(text, &ParseOptions::default()).ok()?; + let root_object = root.object_value_or_create()?; + let imports_object = root_object.object_value_or_create("imports")?; + + if let Some(existing_prop) = imports_object.get(&import.key) { + let existing_value = existing_prop.value()?; + let CstNode::Leaf(leaf) = existing_value else { + return None; + }; + let CstLeafNode::StringLit(string_lit) = leaf else { + return None; + }; + if string_lit.decoded_value().ok()?.as_str() == import.value.as_str() { + return Some(text.to_string()); + } + return None; + } + + imports_object + .append(&import.key, CstInputValue::String(import.value.clone())); + Some(root.to_string()) +} + +fn rewrite_with_import_map_target( + specifier_text: &str, + entries: &[ImportMapEntry], +) -> Option { + entries + .iter() + .filter_map(|entry| { + exact_or_prefix_match(specifier_text, entry) + .map(|replacement| (entry.value.len(), replacement)) + }) + .max_by_key(|(match_len, _)| *match_len) + .map(|(_, replacement)| replacement) +} + +fn exact_or_prefix_match( + specifier_text: &str, + entry: &ImportMapEntry, +) -> Option { + if specifier_text == entry.value { + return Some(entry.key.trim_end_matches('/').to_string()); + } + + if entry.key.ends_with('/') + && entry.value.ends_with('/') + && specifier_text.starts_with(&entry.value) + { + return Some(format!( + "{}{}", + entry.key, + &specifier_text[entry.value.len()..] + )); + } + + None +} + +fn rewrite_with_package_match( + specifier_text: &str, + entries: &[ImportMapEntry], +) -> Option { + let input = PackageSpecifier::parse(specifier_text)?; + entries + .iter() + .filter_map(|entry| { + let target = PackageSpecifier::parse(entry.value.trim_end_matches('/'))?; + if input.kind != target.kind || input.name != target.name { + return None; + } + if target.sub_path.is_some() { + return None; + } + Some(( + entry.key.len(), + package_replacement(entry.key.as_str(), input.sub_path.as_deref()), + )) + }) + .max_by_key(|(match_len, _)| *match_len) + .map(|(_, replacement)| replacement) +} + +fn package_replacement(key: &str, sub_path: Option<&str>) -> String { + let base = key.trim_end_matches('/'); + match sub_path { + Some(sub_path) if key.ends_with('/') => format!("{}{}", key, sub_path), + Some(sub_path) => format!("{}/{}", base, sub_path), + None => base.to_string(), + } +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct PackageSpecifier { + kind: PackageKind, + name: String, + version_req: Option, + sub_path: Option, +} + +#[derive(Clone, Debug, PartialEq, Eq)] +struct SuggestedImport { + key: String, + replacement: String, + value: String, +} + +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum PackageKind { + Jsr, + Npm, +} + +impl PackageKind { + fn scheme(self) -> &'static str { + match self { + Self::Jsr => "jsr:", + Self::Npm => "npm:", + } + } +} + +impl PackageSpecifier { + fn parse(specifier_text: &str) -> Option { + if let Ok(req_ref) = JsrPackageReqReference::from_str(specifier_text) { + return Some(Self::from_req_ref( + PackageKind::Jsr, + req_ref.req().name.as_str(), + req_ref.req().version_req.version_text(), + req_ref.sub_path(), + )); + } + + if let Ok(req_ref) = NpmPackageReqReference::from_str(specifier_text) { + return Some(Self::from_req_ref( + PackageKind::Npm, + req_ref.req().name.as_str(), + req_ref.req().version_req.version_text(), + req_ref.sub_path(), + )); + } + + None + } + + fn from_req_ref( + kind: PackageKind, + name: &str, + version_text: &str, + sub_path: Option<&str>, + ) -> Self { + Self { + kind, + name: name.to_string(), + version_req: (version_text != "*").then(|| version_text.to_string()), + sub_path: sub_path.map(ToString::to_string), + } + } + + fn is_unversioned(&self) -> bool { + self.version_req.is_none() + } + + fn suggested_import( + &self, + lockfile: &LockfileData, + ) -> Option { + let version_req = match &self.version_req { + Some(version_req) => lockfile + .has_package(self.kind, &self.name) + .then_some(version_req.as_str())?, + None => lockfile.unique_version(self.kind, &self.name)?, + }; + let key = if self.sub_path.is_some() { + format!("{}/", self.name) + } else { + self.name.clone() + }; + let replacement = package_replacement(&key, self.sub_path.as_deref()); + let value = if self.sub_path.is_some() { + format!("{}{}@{}/", self.kind.scheme(), self.name, version_req) + } else { + format!("{}{}@{}", self.kind.scheme(), self.name, version_req) + }; + Some(SuggestedImport { + key, + replacement, + value, + }) + } +} + +impl LockfileData { + fn has_package(&self, kind: PackageKind, name: &str) -> bool { + self + .resolved_packages + .iter() + .any(|package| package.kind == kind && package.name == name) + } + + fn unique_version(&self, kind: PackageKind, name: &str) -> Option<&str> { + let mut matches = self + .resolved_packages + .iter() + .filter(|package| package.kind == kind && package.name == name) + .map(|package| package.version.as_str()) + .collect::>(); + matches.sort_unstable(); + matches.dedup(); + if matches.len() == 1 { + matches.into_iter().next() + } else { + None + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn entry(key: &str, value: &str) -> ImportMapEntry { + ImportMapEntry { + key: key.to_string(), + value: value.to_string(), + } + } + + #[test] + fn rewrites_exact_import_map_targets() { + let entries = vec![entry("@std/assert", "jsr:@std/assert@^1.0.0")]; + assert_eq!( + rewrite_with_import_map_target("jsr:@std/assert@^1.0.0", &entries), + Some("@std/assert".to_string()) + ); + } + + #[test] + fn rewrites_import_map_prefix_targets() { + let entries = vec![entry("@std/assert/", "jsr:@std/assert@^1.0.0/")]; + assert_eq!( + rewrite_with_import_map_target( + "jsr:@std/assert@^1.0.0/fmt/colors.ts", + &entries + ), + Some("@std/assert/fmt/colors.ts".to_string()) + ); + } + + #[test] + fn rewrites_package_matches_ignoring_versions() { + let entries = vec![entry("@std/assert", "jsr:@std/assert@^1.0.0")]; + assert_eq!( + rewrite_with_package_match("jsr:@std/assert", &entries), + Some("@std/assert".to_string()) + ); + assert_eq!( + rewrite_with_package_match("jsr:@std/assert/fmt/colors.ts", &entries), + Some("@std/assert/fmt/colors.ts".to_string()) + ); + } + + #[test] + fn uses_lockfile_version_for_unversioned_package() { + let package = PackageSpecifier::parse("jsr:@std/expect").unwrap(); + let lockfile = parse_lockfile( + r#"{ + "version": "5", + "specifiers": { + "jsr:@std/expect@*": "jsr:@std/expect@1.0.0" + } +}"#, + ) + .unwrap(); + assert_eq!( + package.suggested_import(&lockfile), + Some(SuggestedImport { + key: "@std/expect".to_string(), + replacement: "@std/expect".to_string(), + value: "jsr:@std/expect@1.0.0".to_string(), + }) + ); + } + + #[test] + fn uses_lockfile_version_for_sub_path() { + let package = PackageSpecifier::parse("jsr:@std/expect/colors.ts").unwrap(); + let lockfile = parse_lockfile( + r#"{ + "version": "5", + "jsr": { + "@std/expect@1.0.0": { + "integrity": "test" + } + } +}"#, + ) + .unwrap(); + assert_eq!( + package.suggested_import(&lockfile), + Some(SuggestedImport { + key: "@std/expect/".to_string(), + replacement: "@std/expect/colors.ts".to_string(), + value: "jsr:@std/expect@1.0.0/".to_string(), + }) + ); + } + + #[test] + fn requires_unique_lockfile_version() { + let package = PackageSpecifier::parse("jsr:@std/expect").unwrap(); + let lockfile = parse_lockfile( + r#"{ + "version": "5", + "jsr": { + "@std/expect@1.0.0": { + "integrity": "test" + }, + "@std/expect@1.1.0": { + "integrity": "test" + } + } +}"#, + ) + .unwrap(); + assert_eq!(package.suggested_import(&lockfile), None); + } + + #[test] + fn adds_import_to_jsonc_file() { + let updated = add_import_to_config( + "{\n // comment\n}\n", + &SuggestedImport { + key: "@std/expect".to_string(), + replacement: "@std/expect".to_string(), + value: "jsr:@std/expect@^1".to_string(), + }, + ) + .unwrap(); + + assert!(updated.contains("// comment")); + assert!(updated.contains("\"imports\": {")); + assert!(updated.contains("\"@std/expect\": \"jsr:@std/expect@^1\"")); + } +} diff --git a/src/rules/jsx_boolean_value.rs b/src/rules/jsx_boolean_value.rs index f545f1da..a30dce62 100644 --- a/src/rules/jsx_boolean_value.rs +++ b/src/rules/jsx_boolean_value.rs @@ -58,6 +58,7 @@ impl Handler for JSXBooleanValueHandler { fixes.push(LintFix { description: FIX_DESC.into(), changes: vec![LintFixChange { + specifier: None, new_text: "".into(), range: SourceRange::new(start_pos, expr.end()), }], diff --git a/src/rules/jsx_curly_braces.rs b/src/rules/jsx_curly_braces.rs index e865c4b1..967b45a1 100644 --- a/src/rules/jsx_curly_braces.rs +++ b/src/rules/jsx_curly_braces.rs @@ -111,6 +111,7 @@ impl Handler for JSXCurlyBracesHandler { vec![LintFix { description: "Remove curly braces around JSX child".into(), changes: vec![LintFixChange { + specifier: None, new_text: lit_str.value().to_string_lossy().into_owned().into(), range: child.range(), }], @@ -135,6 +136,7 @@ impl Handler for JSXCurlyBracesHandler { description: "Remove curly braces around JSX attribute value" .into(), changes: vec![LintFixChange { + specifier: None, new_text: format!( "\"{}\"", lit_str.value().to_string_lossy() @@ -155,6 +157,7 @@ impl Handler for JSXCurlyBracesHandler { vec![LintFix { description: "Add curly braces around JSX attribute value".into(), changes: vec![LintFixChange { + specifier: None, new_text: format!("{{{}}}", jsx_el.text()).into(), range: value.range(), }], diff --git a/src/rules/jsx_no_unescaped_entities.rs b/src/rules/jsx_no_unescaped_entities.rs index c8dceee9..f4fd6cc6 100644 --- a/src/rules/jsx_no_unescaped_entities.rs +++ b/src/rules/jsx_no_unescaped_entities.rs @@ -52,6 +52,7 @@ impl Handler for JSXNoUnescapedEntitiesHandler { vec![LintFix { description: "Escape entities in the text node".into(), changes: vec![LintFixChange { + specifier: None, new_text: new_text.into(), range: child.range(), }], diff --git a/src/rules/jsx_props_no_spread_multi.rs b/src/rules/jsx_props_no_spread_multi.rs index ff1b4896..56071e50 100644 --- a/src/rules/jsx_props_no_spread_multi.rs +++ b/src/rules/jsx_props_no_spread_multi.rs @@ -57,6 +57,7 @@ impl Handler for JSXPropsNoSpreadMultiHandler { vec![LintFix { description: "Remove this spread attribute".into(), changes: vec![LintFixChange { + specifier: None, new_text: "".into(), range: SourceRange { start: attr.range().start - 2, diff --git a/src/rules/no_import_prefix.rs b/src/rules/no_import_prefix.rs index 27165a0e..b43e725c 100644 --- a/src/rules/no_import_prefix.rs +++ b/src/rules/no_import_prefix.rs @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use super::import_config::fix_with_deno_config_import; use super::{Context, LintRule}; use crate::handler::{Handler, Traverse}; use crate::tags::{Tags, WORKSPACE}; @@ -14,6 +15,7 @@ const CODE: &str = "no-import-prefix"; const MESSAGE: &str = "Inline 'npm:', 'jsr:' or 'https:' dependency not allowed"; const HINT: &str = "Add it as a dependency in a deno.json or package.json instead and reference it here via its bare specifier"; +const FIX_DESCRIPTION: &str = "Use the dependency declared in deno.json"; impl LintRule for NoImportPrefix { fn tags(&self) -> Tags { @@ -37,8 +39,22 @@ struct NoImportPrefixHandler; impl Handler for NoImportPrefixHandler { fn import_decl(&mut self, node: &ImportDecl, ctx: &mut Context) { - if is_non_bare(&node.src.value().to_string_lossy()) { - ctx.add_diagnostic_with_hint(node.src.range(), CODE, MESSAGE, HINT); + let specifier_text = node.src.value().to_string_lossy(); + if is_non_bare(&specifier_text) { + ctx.add_warning_with_fixes( + node.src.range(), + CODE, + MESSAGE, + Some(HINT.to_string()), + fix_with_deno_config_import( + ctx, + node.src.range(), + &specifier_text, + FIX_DESCRIPTION, + ) + .into_iter() + .collect(), + ); } } @@ -46,8 +62,22 @@ impl Handler for NoImportPrefixHandler { if let Callee::Import(_) = node.callee { if let Some(arg) = node.args.first() { if let Expr::Lit(Lit::Str(lit)) = arg.expr { - if is_non_bare(&lit.value().to_string_lossy()) { - ctx.add_diagnostic_with_hint(arg.range(), CODE, MESSAGE, HINT); + let specifier_text = lit.value().to_string_lossy(); + if is_non_bare(&specifier_text) { + ctx.add_warning_with_fixes( + arg.range(), + CODE, + MESSAGE, + Some(HINT.to_string()), + fix_with_deno_config_import( + ctx, + arg.range(), + &specifier_text, + FIX_DESCRIPTION, + ) + .into_iter() + .collect(), + ); } } } @@ -65,6 +95,60 @@ fn is_non_bare(s: &str) -> bool { #[cfg(test)] mod tests { use super::*; + use crate::diagnostic::LintDiagnosticSeverity; + use crate::linter::{LintConfig, LintFileOptions, Linter, LinterOptions}; + use crate::test_util::apply_first_fixes; + use deno_ast::MediaType; + use deno_ast::ModuleSpecifier; + use std::borrow::Cow; + use std::collections::HashSet; + use std::sync::atomic::{AtomicUsize, Ordering}; + + fn filename_with_project( + config_text: &str, + lockfile_text: Option<&str>, + ) -> String { + static NEXT_ID: AtomicUsize = AtomicUsize::new(1); + + let dir = std::env::temp_dir().join(format!( + "deno_lint_no_import_prefix_{}_{}", + std::process::id(), + NEXT_ID.fetch_add(1, Ordering::Relaxed) + )); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("deno.json"), config_text).unwrap(); + if let Some(lockfile_text) = lockfile_text { + std::fs::write(dir.join("deno.lock"), lockfile_text).unwrap(); + } + ModuleSpecifier::from_file_path(dir.join("mod.ts")) + .unwrap() + .to_string() + } + + fn lint_source( + source: &str, + filename: &str, + ) -> Vec { + let linter = Linter::new(LinterOptions { + rules: vec![Box::new(NoImportPrefix)], + all_rule_codes: HashSet::from([Cow::Borrowed(CODE)]), + custom_ignore_file_directive: None, + custom_ignore_diagnostic_directive: None, + }); + linter + .lint_file(LintFileOptions { + specifier: ModuleSpecifier::parse(filename).unwrap(), + source_code: source.to_string(), + media_type: MediaType::TypeScript, + config: LintConfig { + default_jsx_factory: None, + default_jsx_fragment_factory: None, + }, + external_linter: None, + }) + .unwrap() + .1 + } #[test] fn no_with_valid() { @@ -90,43 +174,128 @@ mod tests { r#"import foo from "jsr:@foo/foo";"#: [{ col: 16, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import foo from "npm:foo";"#: [{ col: 16, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import foo from "http://example.com/foo";"#: [{ col: 16, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import foo from "https://example.com/foo";"#: [{ col: 16, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import("jsr:@foo/foo");"#: [{ col: 7, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import("npm:foo");"#: [{ col: 7, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import("http://example.com/foo");"#: [{ col: 7, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import("https://example.com/foo");"#: [{ col: 7, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning + }], + } + } + + #[test] + fn no_import_prefix_is_autofixable_from_deno_json_imports() { + let filename = filename_with_project( + r#"{ + "imports": { + "@std/assert/": "jsr:@std/assert@^1.0.0/" + } +}"#, + None, + ); + + assert_lint_err! { + NoImportPrefix, + filename: filename, + r#"import foo from "jsr:@std/assert@^1.0.0/fmt/colors.ts";"#: [{ + col: 16, + message: MESSAGE, + hint: HINT, + severity: LintDiagnosticSeverity::Warning, + fix: ( + FIX_DESCRIPTION, + r#"import foo from "@std/assert/fmt/colors.ts";"#, + ) }], } } + + #[test] + fn no_import_prefix_does_not_autofix_without_lockfile() { + let filename = filename_with_project("{}\n", None); + let diagnostics = + lint_source(r#"import foo from "jsr:@std/expect@^1";"#, &filename); + assert!(diagnostics + .iter() + .all(|diagnostic| diagnostic.details.fixes.is_empty())); + } + + #[test] + fn no_import_prefix_adds_missing_import_to_deno_json() { + let filename = filename_with_project( + "{}\n", + Some( + r#"{ + "version": "5", + "specifiers": { + "jsr:@std/expect@^1": "jsr:@std/expect@1.0.0" + } +}"#, + ), + ); + let source = r#"import foo from "jsr:@std/expect@^1";"#; + let config_specifier = ModuleSpecifier::parse(&filename) + .unwrap() + .to_file_path() + .unwrap() + .parent() + .unwrap() + .join("deno.json"); + let config_specifier = ModuleSpecifier::from_file_path(config_specifier) + .unwrap() + .to_string(); + + let diagnostics = lint_source(source, &filename); + let updated = apply_first_fixes( + &[ + (filename.as_str(), source), + (config_specifier.as_str(), "{}\n"), + ], + &diagnostics, + ); + + assert_eq!(updated[&filename], r#"import foo from "@std/expect";"#); + assert!(updated[&config_specifier].contains("\"imports\": {")); + assert!(updated[&config_specifier] + .contains("\"@std/expect\": \"jsr:@std/expect@^1\"")); + } } diff --git a/src/rules/no_node_globals.rs b/src/rules/no_node_globals.rs index f074bd20..e04616c2 100644 --- a/src/rules/no_node_globals.rs +++ b/src/rules/no_node_globals.rs @@ -149,6 +149,7 @@ impl NoNodeGlobalsHandler { (range, AddNewline::None) }; LintFixChange { + specifier: None, new_text: fix_kind.to_text(add_newline), range: fix_range, } diff --git a/src/rules/no_process_global.rs b/src/rules/no_process_global.rs index edad5e4f..d86b68e7 100644 --- a/src/rules/no_process_global.rs +++ b/src/rules/no_process_global.rs @@ -76,6 +76,7 @@ impl NoProcessGlobalHandler { }; LintFixChange { + specifier: None, new_text: format!( "{leading}import process from \"node:process\";{trailing}" ) diff --git a/src/rules/no_unversioned_import.rs b/src/rules/no_unversioned_import.rs index 158b710e..180b892b 100644 --- a/src/rules/no_unversioned_import.rs +++ b/src/rules/no_unversioned_import.rs @@ -1,5 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. +use super::import_config::fix_with_deno_config_package; use super::{Context, LintRule}; use crate::handler::{Handler, Traverse}; use crate::tags::{Tags, RECOMMENDED}; @@ -13,6 +14,7 @@ pub struct NoUnversionedImport; const CODE: &str = "no-unversioned-import"; const MESSAGE: &str = "Missing version in specifier"; const HINT: &str = "Add a version requirement after the package name"; +const FIX_DESCRIPTION: &str = "Use the versioned dependency from deno.json"; impl LintRule for NoUnversionedImport { fn tags(&self) -> Tags { @@ -36,8 +38,22 @@ struct NoUnversionedImportHandler; impl Handler for NoUnversionedImportHandler { fn import_decl(&mut self, node: &ImportDecl, ctx: &mut Context) { - if is_unversioned(&node.src.value().to_string_lossy()) { - ctx.add_diagnostic_with_hint(node.src.range(), CODE, MESSAGE, HINT); + let specifier_text = node.src.value().to_string_lossy(); + if is_unversioned(&specifier_text) { + ctx.add_warning_with_fixes( + node.src.range(), + CODE, + MESSAGE, + Some(HINT.to_string()), + fix_with_deno_config_package( + ctx, + node.src.range(), + &specifier_text, + FIX_DESCRIPTION, + ) + .into_iter() + .collect(), + ); } } @@ -45,8 +61,22 @@ impl Handler for NoUnversionedImportHandler { if let Callee::Import(_) = node.callee { if let Some(arg) = node.args.first() { if let Expr::Lit(Lit::Str(lit)) = arg.expr { - if is_unversioned(&lit.value().to_string_lossy()) { - ctx.add_diagnostic_with_hint(arg.range(), CODE, MESSAGE, HINT); + let specifier_text = lit.value().to_string_lossy(); + if is_unversioned(&specifier_text) { + ctx.add_warning_with_fixes( + arg.range(), + CODE, + MESSAGE, + Some(HINT.to_string()), + fix_with_deno_config_package( + ctx, + arg.range(), + &specifier_text, + FIX_DESCRIPTION, + ) + .into_iter() + .collect(), + ); } } } @@ -79,6 +109,63 @@ fn get_package_req_ref( #[cfg(test)] mod tests { use super::*; + use crate::diagnostic::LintDiagnosticSeverity; + use crate::linter::{LintConfig, LintFileOptions, Linter, LinterOptions}; + use crate::rules::no_import_prefix::NoImportPrefix; + use crate::test_util::apply_first_fixes; + use deno_ast::MediaType; + use deno_ast::ModuleSpecifier; + use std::borrow::Cow; + use std::collections::HashSet; + use std::sync::atomic::{AtomicUsize, Ordering}; + + fn filename_with_project( + config_text: &str, + lockfile_text: Option<&str>, + ) -> String { + static NEXT_ID: AtomicUsize = AtomicUsize::new(1); + + let dir = std::env::temp_dir().join(format!( + "deno_lint_no_unversioned_import_{}_{}", + std::process::id(), + NEXT_ID.fetch_add(1, Ordering::Relaxed) + )); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("deno.json"), config_text).unwrap(); + if let Some(lockfile_text) = lockfile_text { + std::fs::write(dir.join("deno.lock"), lockfile_text).unwrap(); + } + ModuleSpecifier::from_file_path(dir.join("mod.ts")) + .unwrap() + .to_string() + } + + fn lint_with_rules( + source: &str, + filename: &str, + rules: Vec>, + all_rule_codes: HashSet>, + ) -> Vec { + let linter = Linter::new(LinterOptions { + rules, + all_rule_codes, + custom_ignore_file_directive: None, + custom_ignore_diagnostic_directive: None, + }); + linter + .lint_file(LintFileOptions { + specifier: ModuleSpecifier::parse(filename).unwrap(), + source_code: source.to_string(), + media_type: MediaType::TypeScript, + config: LintConfig { + default_jsx_factory: None, + default_jsx_fragment_factory: None, + }, + external_linter: None, + }) + .unwrap() + .1 + } #[test] fn no_with_valid() { @@ -117,33 +204,183 @@ mod tests { r#"import foo from "jsr:@foo/foo";"#: [{ col: 16, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import foo from "npm:foo";"#: [{ col: 16, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import foo from "npm:@foo/bar";"#: [{ col: 16, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import("jsr:@foo/foo");"#: [{ col: 7, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import("npm:foo");"#: [{ col: 7, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning }], r#"import("npm:@foo/bar");"#: [{ col: 7, message: MESSAGE, - hint: HINT + hint: HINT, + severity: LintDiagnosticSeverity::Warning + }], + } + } + + #[test] + fn no_unversioned_import_is_autofixable_from_deno_json_imports() { + let filename = filename_with_project( + r#"{ + "imports": { + "@std/assert": "jsr:@std/assert@^1.0.0" + } +}"#, + None, + ); + + assert_lint_err! { + NoUnversionedImport, + filename: filename, + r#"import foo from "jsr:@std/assert";"#: [{ + col: 16, + message: MESSAGE, + hint: HINT, + severity: LintDiagnosticSeverity::Warning, + fix: ( + FIX_DESCRIPTION, + r#"import foo from "@std/assert";"#, + ) }], } } + + #[test] + fn no_unversioned_import_does_not_autofix_without_lockfile() { + let filename = filename_with_project("{}\n", None); + let diagnostics = lint_with_rules( + r#"import foo from "jsr:@std/expect";"#, + &filename, + vec![Box::new(NoUnversionedImport)], + HashSet::from([Cow::Borrowed(CODE)]), + ); + assert!(diagnostics + .iter() + .all(|diagnostic| diagnostic.details.fixes.is_empty())); + } + + #[test] + fn no_unversioned_import_adds_missing_import_to_deno_json() { + let filename = filename_with_project( + "{}\n", + Some( + r#"{ + "version": "5", + "specifiers": { + "jsr:@std/expect@*": "jsr:@std/expect@1.0.0" + } +}"#, + ), + ); + let source = r#"import foo from "jsr:@std/expect";"#; + let config_specifier = ModuleSpecifier::parse(&filename) + .unwrap() + .to_file_path() + .unwrap() + .parent() + .unwrap() + .join("deno.json"); + let config_specifier = ModuleSpecifier::from_file_path(config_specifier) + .unwrap() + .to_string(); + + let diagnostics = lint_with_rules( + source, + &filename, + vec![Box::new(NoUnversionedImport)], + HashSet::from([Cow::Borrowed(CODE)]), + ); + let updated = apply_first_fixes( + &[ + (filename.as_str(), source), + (config_specifier.as_str(), "{}\n"), + ], + &diagnostics, + ); + + assert_eq!(updated[&filename], r#"import foo from "@std/expect";"#); + assert!(updated[&config_specifier] + .contains("\"@std/expect\": \"jsr:@std/expect@1.0.0\"")); + } + + #[test] + fn combined_warnings_apply_cleanly() { + let filename = filename_with_project( + "{}\n", + Some( + r#"{ + "version": "5", + "specifiers": { + "jsr:@std/expect@*": "jsr:@std/expect@1.0.0" + } +}"#, + ), + ); + let source = r#"import foo from "jsr:@std/expect";"#; + let config_specifier = ModuleSpecifier::parse(&filename) + .unwrap() + .to_file_path() + .unwrap() + .parent() + .unwrap() + .join("deno.json"); + let config_specifier = ModuleSpecifier::from_file_path(config_specifier) + .unwrap() + .to_string(); + + let diagnostics = lint_with_rules( + source, + &filename, + vec![Box::new(NoImportPrefix), Box::new(NoUnversionedImport)], + HashSet::from([Cow::Borrowed("no-import-prefix"), Cow::Borrowed(CODE)]), + ); + + assert_eq!(diagnostics.len(), 2); + assert_eq!(diagnostics[0].severity(), LintDiagnosticSeverity::Warning); + assert_eq!(diagnostics[1].severity(), LintDiagnosticSeverity::Warning); + assert!(diagnostics + .iter() + .any(|diagnostic| diagnostic.details.fixes.is_empty())); + assert!(diagnostics.iter().any(|diagnostic| { + diagnostic + .details + .fixes + .first() + .is_some_and(|fix| fix.changes.len() == 2) + })); + + let updated = apply_first_fixes( + &[ + (filename.as_str(), source), + (config_specifier.as_str(), "{}\n"), + ], + &diagnostics, + ); + + assert_eq!(updated[&filename], r#"import foo from "@std/expect";"#); + assert!(updated[&config_specifier] + .contains("\"@std/expect\": \"jsr:@std/expect@1.0.0\"")); + } } diff --git a/src/rules/no_window.rs b/src/rules/no_window.rs index a7f9097a..ff6d76fd 100644 --- a/src/rules/no_window.rs +++ b/src/rules/no_window.rs @@ -52,6 +52,7 @@ impl NoWindowGlobalHandler { vec![LintFix { description: FIX_DESC.into(), changes: vec![LintFixChange { + specifier: None, new_text: "globalThis".into(), range, }], diff --git a/src/rules/no_window_prefix.rs b/src/rules/no_window_prefix.rs index 36578d7d..b38fdb10 100644 --- a/src/rules/no_window_prefix.rs +++ b/src/rules/no_window_prefix.rs @@ -256,6 +256,7 @@ impl Handler for NoWindowPrefixHandler { vec![LintFix { description: FIX_DESC.into(), changes: vec![LintFixChange { + specifier: None, new_text: "globalThis".into(), range: obj_ident.range(), }], diff --git a/src/rules/verbatim_module_syntax.rs b/src/rules/verbatim_module_syntax.rs index 1dd3cab6..178ae43d 100644 --- a/src/rules/verbatim_module_syntax.rs +++ b/src/rules/verbatim_module_syntax.rs @@ -97,6 +97,7 @@ impl VerbatimModuleSyntax { let import_token_range = import.tokens_fast(program)[0].range(); let mut changes = Vec::with_capacity(1 + type_only_named_import.len()); changes.push(LintFixChange { + specifier: None, new_text: " type".into(), range: import_token_range.end().range(), }); @@ -105,6 +106,7 @@ impl VerbatimModuleSyntax { let tokens = named_import.tokens_fast(program); let range = SourceRange::new(tokens[0].start(), tokens[1].start()); changes.push(LintFixChange { + specifier: None, new_text: "".into(), range, }); @@ -129,6 +131,7 @@ impl VerbatimModuleSyntax { vec![LintFix { description: FIX_ADD_TYPE_KEYWORD_DESC.into(), changes: vec![LintFixChange { + specifier: None, new_text: "type ".into(), range: specifier.start().range(), }], @@ -162,6 +165,7 @@ impl VerbatimModuleSyntax { "" }; let changes = Vec::from([LintFixChange { + specifier: None, new_text: format!( "import {0}{1}{0}{2}", quote_kind, @@ -216,6 +220,7 @@ impl VerbatimModuleSyntax { let export_token_range = named_export.tokens_fast(program)[0].range(); let mut changes = Vec::with_capacity(1 + type_only_named_export.len()); changes.push(LintFixChange { + specifier: None, new_text: " type".into(), range: export_token_range.end().range(), }); @@ -224,6 +229,7 @@ impl VerbatimModuleSyntax { let tokens = named_import.tokens_fast(program); let range = SourceRange::new(tokens[0].start(), tokens[1].start()); changes.push(LintFixChange { + specifier: None, new_text: "".into(), range, }); @@ -248,6 +254,7 @@ impl VerbatimModuleSyntax { vec![LintFix { description: FIX_ADD_TYPE_KEYWORD_DESC.into(), changes: vec![LintFixChange { + specifier: None, new_text: "type ".into(), range: specifier.start().range(), }], diff --git a/src/test_util.rs b/src/test_util.rs index 3746354b..87b1c8e7 100644 --- a/src/test_util.rs +++ b/src/test_util.rs @@ -1,9 +1,10 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use std::borrow::Cow; +use std::collections::HashMap; use crate::ast_parser; -use crate::diagnostic::LintDiagnostic; +use crate::diagnostic::{LintDiagnostic, LintDiagnosticSeverity}; use crate::linter::LintConfig; use crate::linter::LintFileOptions; use crate::linter::Linter; @@ -183,7 +184,7 @@ macro_rules! parse_err_test { pub struct LintErrTester { src: &'static str, errors: Vec, - filename: &'static str, + filename: String, rule: Box, } @@ -192,12 +193,12 @@ impl LintErrTester { rule: Box, src: &'static str, errors: Vec, - filename: &'static str, + filename: impl Into, ) -> Self { Self { src, errors, - filename, + filename: filename.into(), rule, } } @@ -205,7 +206,8 @@ impl LintErrTester { #[track_caller] pub fn run(self) { let rule_code = self.rule.code(); - let (parsed_source, diagnostics) = lint(self.rule, self.src, self.filename); + let (parsed_source, diagnostics) = + lint(self.rule, self.src, &self.filename); if self.errors.len() != diagnostics.len() { eprintln!( "Actual diagnostics:\n{:#?}", @@ -231,6 +233,7 @@ impl LintErrTester { message, hint, fixes, + severity, } = error; assert_diagnostic_2( diagnostic, @@ -241,6 +244,7 @@ impl LintErrTester { message, hint.as_deref(), fixes, + *severity, parsed_source.text_info_lazy(), ); } @@ -260,6 +264,7 @@ pub struct LintErr { pub message: String, pub hint: Option, pub fixes: Vec, + pub severity: LintDiagnosticSeverity, } #[derive(Default)] @@ -269,6 +274,7 @@ pub struct LintErrBuilder { message: Option, hint: Option, fixes: Vec, + severity: Option, } impl LintErrBuilder { @@ -306,6 +312,11 @@ impl LintErrBuilder { self } + pub fn severity(&mut self, severity: LintDiagnosticSeverity) -> &mut Self { + self.severity = Some(severity); + self + } + pub fn build(self) -> LintErr { LintErr { line: self.line.unwrap_or(1), @@ -313,6 +324,7 @@ impl LintErrBuilder { message: self.message.unwrap_or_default(), hint: self.hint, fixes: self.fixes, + severity: self.severity.unwrap_or(LintDiagnosticSeverity::Error), } } } @@ -396,6 +408,7 @@ fn assert_diagnostic_2( message: &str, hint: Option<&str>, fixes: &[LintErrFix], + severity: LintDiagnosticSeverity, text_info: &SourceTextInfo, ) { let diagnostic_range = diagnostic.range.as_ref().unwrap(); @@ -433,6 +446,14 @@ fn assert_diagnostic_2( diagnostic.details.hint.as_deref(), source ); + assert_eq!( + severity, + diagnostic.severity(), + "Diagnostic severity is expected to be \"{:?}\", but got \"{:?}\"\n\nsource:\n{}\n", + severity, + diagnostic.severity(), + source + ); let actual_fixes = diagnostic .details .fixes @@ -444,6 +465,12 @@ fn assert_diagnostic_2( fix .changes .iter() + .filter(|change| { + change + .specifier + .as_ref() + .is_none_or(|specifier| specifier == &diagnostic.specifier) + }) .map(|change| TextChange { range: change.range.as_byte_range(text_info.range().start), new_text: change.new_text.to_string(), @@ -456,11 +483,7 @@ fn assert_diagnostic_2( } #[track_caller] -pub fn assert_lint_ok( - rule: Box, - source: &str, - specifier: &'static str, -) { +pub fn assert_lint_ok(rule: Box, source: &str, specifier: &str) { let (_parsed_source, diagnostics) = lint(rule, source, specifier); if !diagnostics.is_empty() { eprintln!("filename {:?}", specifier); @@ -488,6 +511,67 @@ pub fn parse(source_code: &str) -> ParsedSource { .unwrap() } +pub fn apply_first_fixes( + files: &[(&str, &str)], + diagnostics: &[LintDiagnostic], +) -> HashMap { + let mut file_texts = files + .iter() + .map(|(specifier, text)| (specifier.to_string(), text.to_string())) + .collect::>(); + + let mut changes_by_specifier = HashMap::>::new(); + for diagnostic in diagnostics { + let Some(fix) = diagnostic.details.fixes.first() else { + continue; + }; + + for change in &fix.changes { + let target_specifier = change + .specifier + .as_ref() + .unwrap_or(&diagnostic.specifier) + .to_string(); + let text = file_texts.get(&target_specifier).unwrap_or_else(|| { + panic!("Missing file contents for {}", target_specifier) + }); + let text_info = SourceTextInfo::new(text.clone().into()); + changes_by_specifier + .entry(target_specifier) + .or_default() + .push(TextChange { + range: change.range.as_byte_range(text_info.range().start), + new_text: change.new_text.to_string(), + }); + } + } + + for (specifier, changes) in changes_by_specifier { + let source = file_texts.remove(&specifier).unwrap(); + let updated = + deno_ast::apply_text_changes(&source, dedupe_text_changes(changes)); + file_texts.insert(specifier, updated); + } + + file_texts +} + +fn dedupe_text_changes(mut changes: Vec) -> Vec { + changes.sort_by(|a, b| match a.range.start.cmp(&b.range.start) { + std::cmp::Ordering::Equal => match a.range.end.cmp(&b.range.end) { + std::cmp::Ordering::Equal => a.new_text.cmp(&b.new_text), + ordering => ordering, + }, + ordering => ordering, + }); + changes.dedup_by(|a, b| { + a.range.start == b.range.start + && a.range.end == b.range.end + && a.new_text == b.new_text + }); + changes +} + pub fn parse_and_then(source_code: &str, test: impl Fn(ast_view::Program)) { let parsed_source = parse(source_code); parsed_source.with_view(|pg| {