From 2a026bee33e829706742a2abed36a71193d3d461 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 25 Jul 2024 09:07:59 -0400 Subject: [PATCH] fix(unstable): move sloppy-import warnings to lint rule (#24710) Adds a new `no-sloppy-imports` lint rule and cleans up the lint code. Closes #22844 Closes https://github.com/denoland/deno_lint/issues/1293 --- Cargo.lock | 4 +- cli/Cargo.toml | 2 +- cli/factory.rs | 32 +- cli/graph_util.rs | 4 +- cli/lsp/analysis.rs | 66 +- cli/lsp/config.rs | 45 +- cli/lsp/diagnostics.rs | 63 +- cli/lsp/resolver.rs | 8 +- cli/resolver.rs | 244 +++----- cli/standalone/mod.rs | 11 +- cli/tools/lint/linter.rs | 242 ++++++++ cli/tools/lint/mod.rs | 584 ++++-------------- cli/tools/lint/no_slow_types.rs | 38 -- cli/tools/lint/rules/mod.rs | 296 +++++++++ cli/tools/lint/rules/no_sloppy_imports.md | 20 + cli/tools/lint/rules/no_sloppy_imports.rs | 214 +++++++ cli/tools/lint/rules/no_slow_types.md | 3 + cli/tools/lint/rules/no_slow_types.rs | 98 +++ cli/tools/registry/mod.rs | 4 +- cli/tools/registry/unfurl.rs | 4 +- cli/tools/run/mod.rs | 7 - tests/integration/check_tests.rs | 102 --- tests/integration/lsp_tests.rs | 90 ++- tests/integration/run_tests.rs | 80 --- .../lint/sloppy_imports_dts/__test__.jsonc | 34 + tests/specs/lint/sloppy_imports_dts/a.d.ts | 1 + tests/specs/lint/sloppy_imports_dts/a.ts | 1 + tests/specs/lint/sloppy_imports_dts/b.d.ts | 1 + tests/specs/lint/sloppy_imports_dts/b.js | 1 + tests/specs/lint/sloppy_imports_dts/c.d.mts | 1 + tests/specs/lint/sloppy_imports_dts/c.mts | 1 + tests/specs/lint/sloppy_imports_dts/check.out | 1 + tests/specs/lint/sloppy_imports_dts/d.d.mts | 1 + tests/specs/lint/sloppy_imports_dts/d.mjs | 1 + .../lint/sloppy_imports_dts/dir_js/index.d.ts | 1 + .../lint/sloppy_imports_dts/dir_js/index.js | 1 + .../sloppy_imports_dts/dir_mjs/index.d.ts | 1 + .../lint/sloppy_imports_dts/dir_mjs/index.mjs | 1 + .../sloppy_imports_dts/dir_mts/index.d.ts | 1 + .../lint/sloppy_imports_dts/dir_mts/index.mts | 1 + .../lint/sloppy_imports_dts/dir_ts/index.d.ts | 1 + .../lint/sloppy_imports_dts/dir_ts/index.ts | 1 + tests/specs/lint/sloppy_imports_dts/lint.out | 110 ++++ tests/specs/lint/sloppy_imports_dts/main.ts | 29 + tests/specs/lint/sloppy_imports_dts/run.out | 12 + .../__test__.jsonc | 20 + .../fail_js_to_ts.out | 11 + .../file.js | 1 + .../main.ts | 3 + .../publish/sloppy_imports/sloppy_imports.out | 2 - tests/specs/run/sloppy_imports/__test__.jsonc | 10 + tests/specs/run/sloppy_imports/a.ts | 1 + tests/specs/run/sloppy_imports/b.js | 1 + tests/specs/run/sloppy_imports/c.mts | 1 + tests/specs/run/sloppy_imports/d.mjs | 1 + tests/specs/run/sloppy_imports/dir/index.tsx | 1 + tests/specs/run/sloppy_imports/e.tsx | 1 + tests/specs/run/sloppy_imports/f.jsx | 1 + tests/specs/run/sloppy_imports/main.ts | 16 + tests/specs/run/sloppy_imports/no_sloppy.out | 2 + tests/specs/run/sloppy_imports/sloppy.out | 8 + .../invalid_unstable_feature.out | 1 - tests/util/server/src/lsp.rs | 12 +- 63 files changed, 1601 insertions(+), 955 deletions(-) create mode 100644 cli/tools/lint/linter.rs delete mode 100644 cli/tools/lint/no_slow_types.rs create mode 100644 cli/tools/lint/rules/mod.rs create mode 100644 cli/tools/lint/rules/no_sloppy_imports.md create mode 100644 cli/tools/lint/rules/no_sloppy_imports.rs create mode 100644 cli/tools/lint/rules/no_slow_types.md create mode 100644 cli/tools/lint/rules/no_slow_types.rs create mode 100644 tests/specs/lint/sloppy_imports_dts/__test__.jsonc create mode 100644 tests/specs/lint/sloppy_imports_dts/a.d.ts create mode 100644 tests/specs/lint/sloppy_imports_dts/a.ts create mode 100644 tests/specs/lint/sloppy_imports_dts/b.d.ts create mode 100644 tests/specs/lint/sloppy_imports_dts/b.js create mode 100644 tests/specs/lint/sloppy_imports_dts/c.d.mts create mode 100644 tests/specs/lint/sloppy_imports_dts/c.mts create mode 100644 tests/specs/lint/sloppy_imports_dts/check.out create mode 100644 tests/specs/lint/sloppy_imports_dts/d.d.mts create mode 100644 tests/specs/lint/sloppy_imports_dts/d.mjs create mode 100644 tests/specs/lint/sloppy_imports_dts/dir_js/index.d.ts create mode 100644 tests/specs/lint/sloppy_imports_dts/dir_js/index.js create mode 100644 tests/specs/lint/sloppy_imports_dts/dir_mjs/index.d.ts create mode 100644 tests/specs/lint/sloppy_imports_dts/dir_mjs/index.mjs create mode 100644 tests/specs/lint/sloppy_imports_dts/dir_mts/index.d.ts create mode 100644 tests/specs/lint/sloppy_imports_dts/dir_mts/index.mts create mode 100644 tests/specs/lint/sloppy_imports_dts/dir_ts/index.d.ts create mode 100644 tests/specs/lint/sloppy_imports_dts/dir_ts/index.ts create mode 100644 tests/specs/lint/sloppy_imports_dts/lint.out create mode 100644 tests/specs/lint/sloppy_imports_dts/main.ts create mode 100644 tests/specs/lint/sloppy_imports_dts/run.out create mode 100644 tests/specs/lint/sloppy_imports_no_incremental_cache/__test__.jsonc create mode 100644 tests/specs/lint/sloppy_imports_no_incremental_cache/fail_js_to_ts.out create mode 100644 tests/specs/lint/sloppy_imports_no_incremental_cache/file.js create mode 100644 tests/specs/lint/sloppy_imports_no_incremental_cache/main.ts create mode 100644 tests/specs/run/sloppy_imports/__test__.jsonc create mode 100644 tests/specs/run/sloppy_imports/a.ts create mode 100644 tests/specs/run/sloppy_imports/b.js create mode 100644 tests/specs/run/sloppy_imports/c.mts create mode 100644 tests/specs/run/sloppy_imports/d.mjs create mode 100644 tests/specs/run/sloppy_imports/dir/index.tsx create mode 100644 tests/specs/run/sloppy_imports/e.tsx create mode 100644 tests/specs/run/sloppy_imports/f.jsx create mode 100644 tests/specs/run/sloppy_imports/main.ts create mode 100644 tests/specs/run/sloppy_imports/no_sloppy.out create mode 100644 tests/specs/run/sloppy_imports/sloppy.out diff --git a/Cargo.lock b/Cargo.lock index 9baddb96b7c800..26c63069f73f70 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1650,9 +1650,9 @@ dependencies = [ [[package]] name = "deno_lint" -version = "0.60.1" +version = "0.61.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "598de34cdfb2a8ed335d8f2e33a75249445a9f81c8092a069fc562c2d5cdb9b6" +checksum = "d127c05c87cb0fa2a59ad9bc70084f06731a5117c14888253269b6e921cfaef1" dependencies = [ "anyhow", "deno_ast", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 4242fb09cdfb09..a19dcbe3d1a77d 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -70,7 +70,7 @@ deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] deno_doc = { version = "0.144.0", features = ["html", "syntect"] } deno_emit = "=0.43.1" deno_graph = { version = "=0.80.1", features = ["tokio_executor"] } -deno_lint = { version = "=0.60.1", features = ["docs"] } +deno_lint = { version = "=0.61.0", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "=0.21.4" deno_package_json.workspace = true diff --git a/cli/factory.rs b/cli/factory.rs index d701c2719cdfba..aeab3cbc45974b 100644 --- a/cli/factory.rs +++ b/cli/factory.rs @@ -45,6 +45,7 @@ use crate::resolver::SloppyImportsResolver; use crate::standalone::DenoCompileBinaryWriter; use crate::tools::check::TypeChecker; use crate::tools::coverage::CoverageCollector; +use crate::tools::lint::LintRuleProvider; use crate::tools::run::hmr::HmrRunner; use crate::util::file_watcher::WatcherCommunicator; use crate::util::fs::canonicalize_path_maybe_not_exists; @@ -179,6 +180,7 @@ struct CliFactoryServices { node_code_translator: Deferred>, node_resolver: Deferred>, npm_resolver: Deferred>, + sloppy_imports_resolver: Deferred>>, text_only_progress_bar: Deferred, type_checker: Deferred>, cjs_resolutions: Deferred>, @@ -397,6 +399,23 @@ impl CliFactory { .await } + pub fn sloppy_imports_resolver( + &self, + ) -> Result>, AnyError> { + self + .services + .sloppy_imports_resolver + .get_or_try_init(|| { + Ok( + self + .cli_options()? + .unstable_sloppy_imports() + .then(|| Arc::new(SloppyImportsResolver::new(self.fs().clone()))), + ) + }) + .map(|maybe| maybe.as_ref()) + } + pub async fn workspace_resolver( &self, ) -> Result<&Arc, AnyError> { @@ -440,11 +459,7 @@ impl CliFactory { async { let cli_options = self.cli_options()?; Ok(Arc::new(CliGraphResolver::new(CliGraphResolverOptions { - sloppy_imports_resolver: if cli_options.unstable_sloppy_imports() { - Some(SloppyImportsResolver::new(self.fs().clone())) - } else { - None - }, + sloppy_imports_resolver: self.sloppy_imports_resolver()?.cloned(), node_resolver: Some(self.cli_node_resolver().await?.clone()), npm_resolver: if cli_options.no_npm() { None @@ -524,6 +539,13 @@ impl CliFactory { }) } + pub async fn lint_rule_provider(&self) -> Result { + Ok(LintRuleProvider::new( + self.sloppy_imports_resolver()?.cloned(), + Some(self.workspace_resolver().await?.clone()), + )) + } + pub async fn node_resolver(&self) -> Result<&Arc, AnyError> { self .services diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 7f664420c5ccf4..91549472ea71aa 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -750,8 +750,8 @@ fn enhanced_sloppy_imports_error_message( ModuleError::LoadingErr(specifier, _, ModuleLoadError::Loader(_)) // ex. "Is a directory" error | ModuleError::Missing(specifier, _) => { let additional_message = SloppyImportsResolver::new(fs.clone()) - .resolve(specifier, ResolutionMode::Execution) - .as_suggestion_message()?; + .resolve(specifier, ResolutionMode::Execution)? + .as_suggestion_message(); Some(format!( "{} {} or run with --unstable-sloppy-imports", error, diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index a6a69cbf0e5885..97730ac7ea5535 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -8,8 +8,8 @@ use super::resolver::LspResolver; use super::tsc; use crate::args::jsr_url; -use crate::tools::lint::create_linter; -use deno_lint::linter::LintConfig; +use crate::tools::lint::CliLinter; +use deno_lint::diagnostic::LintDiagnosticRange; use deno_runtime::fs_util::specifier_to_file_path; use deno_ast::SourceRange; @@ -23,8 +23,6 @@ use deno_core::serde::Serialize; use deno_core::serde_json; use deno_core::serde_json::json; use deno_core::ModuleSpecifier; -use deno_lint::diagnostic::LintDiagnostic; -use deno_lint::rules::LintRule; use deno_runtime::deno_node::NpmResolver; use deno_runtime::deno_node::PathClean; use deno_semver::jsr::JsrPackageNvReference; @@ -149,8 +147,10 @@ impl Reference { } } -fn as_lsp_range_from_diagnostic(diagnostic: &LintDiagnostic) -> Range { - as_lsp_range(diagnostic.range, &diagnostic.text_info) +fn as_lsp_range_from_lint_diagnostic( + diagnostic_range: &LintDiagnosticRange, +) -> Range { + as_lsp_range(diagnostic_range.range, &diagnostic_range.text_info) } fn as_lsp_range( @@ -173,37 +173,39 @@ fn as_lsp_range( pub fn get_lint_references( parsed_source: &deno_ast::ParsedSource, - lint_rules: Vec<&'static dyn LintRule>, - lint_config: LintConfig, + linter: &CliLinter, ) -> Result, AnyError> { - let linter = create_linter(lint_rules); - let lint_diagnostics = linter.lint_with_ast(parsed_source, lint_config); + let lint_diagnostics = linter.lint_with_ast(parsed_source); Ok( lint_diagnostics .into_iter() - .map(|d| Reference { - range: as_lsp_range_from_diagnostic(&d), - category: Category::Lint { - message: d.message, - code: d.code, - hint: d.hint, - quick_fixes: d - .fixes - .into_iter() - .map(|f| DataQuickFix { - description: f.description.to_string(), - changes: f - .changes - .into_iter() - .map(|change| DataQuickFixChange { - range: as_lsp_range(change.range, &d.text_info), - new_text: change.new_text.to_string(), - }) - .collect(), - }) - .collect(), - }, + .filter_map(|d| { + let range = d.range.as_ref()?; + Some(Reference { + range: as_lsp_range_from_lint_diagnostic(range), + category: Category::Lint { + message: d.details.message, + code: d.details.code.to_string(), + hint: d.details.hint, + quick_fixes: d + .details + .fixes + .into_iter() + .map(|f| DataQuickFix { + description: f.description.to_string(), + changes: f + .changes + .into_iter() + .map(|change| DataQuickFixChange { + range: as_lsp_range(change.range, &range.text_info), + new_text: change.new_text.to_string(), + }) + .collect(), + }) + .collect(), + }, + }) }) .collect(), ) diff --git a/cli/lsp/config.rs b/cli/lsp/config.rs index ec5430e8aff1b1..8a44c26ddc9cb5 100644 --- a/cli/lsp/config.rs +++ b/cli/lsp/config.rs @@ -52,12 +52,16 @@ use crate::args::discover_npmrc_from_workspace; use crate::args::has_flag_env_var; use crate::args::CliLockfile; use crate::args::ConfigFile; +use crate::args::LintFlags; +use crate::args::LintOptions; use crate::args::DENO_FUTURE; use crate::cache::FastInsecureHasher; use crate::file_fetcher::FileFetcher; use crate::lsp::logging::lsp_warn; -use crate::tools::lint::get_configured_rules; -use crate::tools::lint::ConfiguredRules; +use crate::resolver::SloppyImportsResolver; +use crate::tools::lint::CliLinter; +use crate::tools::lint::CliLinterOptions; +use crate::tools::lint::LintRuleProvider; use crate::util::fs::canonicalize_path_maybe_not_exists; pub const SETTINGS_SECTION: &str = "deno"; @@ -1116,8 +1120,7 @@ pub struct ConfigData { pub lint_config: Arc, pub test_config: Arc, pub exclude_files: Arc, - pub deno_lint_config: DenoLintConfig, - pub lint_rules: Arc, + pub linter: Arc, pub ts_config: Arc, pub byonm: bool, pub node_modules_dir: Option, @@ -1125,6 +1128,7 @@ pub struct ConfigData { pub lockfile: Option>, pub npmrc: Option>, pub resolver: Arc, + pub sloppy_imports_resolver: Option>, pub import_map_from_settings: Option, watched_files: HashMap, } @@ -1310,10 +1314,7 @@ impl ConfigData { LintConfig::new_with_base(default_file_pattern_base.clone()) }), ); - let lint_rules = Arc::new(get_configured_rules( - lint_config.options.rules.clone(), - member_dir.maybe_deno_json().map(|c| c.as_ref()), - )); + let test_config = Arc::new( member_dir .to_test_config(FilePatterns::new_with_base(member_dir.dir_path())) @@ -1532,16 +1533,38 @@ impl ConfigData { .join("\n") ); } + let unstable_sloppy_imports = std::env::var("DENO_UNSTABLE_SLOPPY_IMPORTS") + .is_ok() + || member_dir.workspace.has_unstable("sloppy-imports"); + let sloppy_imports_resolver = unstable_sloppy_imports.then(|| { + Arc::new(SloppyImportsResolver::new_without_stat_cache(Arc::new( + deno_runtime::deno_fs::RealFs, + ))) + }); + let resolver = Arc::new(resolver); + let lint_rule_provider = LintRuleProvider::new( + sloppy_imports_resolver.clone(), + Some(resolver.clone()), + ); + let linter = Arc::new(CliLinter::new(CliLinterOptions { + configured_rules: lint_rule_provider.resolve_lint_rules( + LintOptions::resolve((*lint_config).clone(), &LintFlags::default()) + .rules, + member_dir.maybe_deno_json().map(|c| c.as_ref()), + ), + fix: false, + deno_lint_config, + })); ConfigData { scope, member_dir, - resolver: Arc::new(resolver), + resolver, + sloppy_imports_resolver, fmt_config, lint_config, test_config, - deno_lint_config, - lint_rules, + linter, exclude_files, ts_config: Arc::new(ts_config), byonm, diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 899dc9681ad751..ed92c400fa83a7 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -20,6 +20,9 @@ use crate::graph_util::enhanced_resolution_error_message; use crate::lsp::lsp_custom::DiagnosticBatchNotificationParams; use crate::resolver::SloppyImportsResolution; use crate::resolver::SloppyImportsResolver; +use crate::tools::lint::CliLinter; +use crate::tools::lint::CliLinterOptions; +use crate::tools::lint::LintRuleProvider; use crate::util::path::to_percent_decoded_str; use deno_ast::MediaType; @@ -40,8 +43,6 @@ use deno_graph::source::ResolveError; use deno_graph::Resolution; use deno_graph::ResolutionError; use deno_graph::SpecifierError; -use deno_lint::linter::LintConfig as DenoLintConfig; -use deno_lint::rules::LintRule; use deno_runtime::deno_fs; use deno_runtime::deno_node; use deno_runtime::tokio_util::create_basic_runtime; @@ -817,25 +818,25 @@ fn generate_lint_diagnostics( continue; } let version = document.maybe_lsp_version(); - let (lint_config, deno_lint_config, lint_rules) = config + let (lint_config, linter) = config .tree .scope_for_specifier(specifier) .and_then(|s| config_data_by_scope.get(s)) - .map(|d| { - ( - d.lint_config.clone(), - d.deno_lint_config.clone(), - d.lint_rules.clone(), - ) - }) + .map(|d| (d.lint_config.clone(), d.linter.clone())) .unwrap_or_else(|| { ( Arc::new(LintConfig::new_with_base(PathBuf::from("/"))), - DenoLintConfig { - default_jsx_factory: None, - default_jsx_fragment_factory: None, - }, - Arc::default(), + Arc::new(CliLinter::new(CliLinterOptions { + configured_rules: { + let lint_rule_provider = LintRuleProvider::new(None, None); + lint_rule_provider.resolve_lint_rules(Default::default(), None) + }, + fix: false, + deno_lint_config: deno_lint::linter::LintConfig { + default_jsx_factory: None, + default_jsx_fragment_factory: None, + }, + })), ) }); diagnostics_vec.push(DiagnosticRecord { @@ -845,8 +846,7 @@ fn generate_lint_diagnostics( diagnostics: generate_document_lint_diagnostics( &document, &lint_config, - deno_lint_config, - lint_rules.rules.clone(), + &linter, ), }, }); @@ -857,19 +857,16 @@ fn generate_lint_diagnostics( fn generate_document_lint_diagnostics( document: &Document, lint_config: &LintConfig, - deno_lint_config: DenoLintConfig, - lint_rules: Vec<&'static dyn LintRule>, + linter: &CliLinter, ) -> Vec { if !lint_config.files.matches_specifier(document.specifier()) { return Vec::new(); } match document.maybe_parsed_source() { Some(Ok(parsed_source)) => { - if let Ok(references) = analysis::get_lint_references( - parsed_source, - lint_rules, - deno_lint_config, - ) { + if let Ok(references) = + analysis::get_lint_references(parsed_source, linter) + { references .into_iter() .map(|r| r.to_diagnostic()) @@ -1237,16 +1234,14 @@ impl DenoDiagnostic { pub fn to_lsp_diagnostic(&self, range: &lsp::Range) -> lsp::Diagnostic { fn no_local_message( specifier: &ModuleSpecifier, - sloppy_resolution: SloppyImportsResolution, + maybe_sloppy_resolution: Option<&SloppyImportsResolution>, ) -> String { let mut message = format!( "Unable to load a local module: {}\n", to_percent_decoded_str(specifier.as_ref()) ); - if let Some(additional_message) = - sloppy_resolution.as_suggestion_message() - { - message.push_str(&additional_message); + if let Some(res) = maybe_sloppy_resolution { + message.push_str(&res.as_suggestion_message()); message.push('.'); } else { message.push_str("Please check the file path."); @@ -1263,15 +1258,15 @@ impl DenoDiagnostic { Self::NoCacheJsr(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing jsr package: {}", pkg_req), Some(json!({ "specifier": specifier }))), Self::NoCacheNpm(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("Uncached or missing npm package: {}", pkg_req), Some(json!({ "specifier": specifier }))), Self::NoLocal(specifier) => { - let sloppy_resolution = SloppyImportsResolver::new(Arc::new(deno_fs::RealFs)).resolve(specifier, ResolutionMode::Execution); - let data = sloppy_resolution.as_lsp_quick_fix_message().map(|message| { + let maybe_sloppy_resolution = SloppyImportsResolver::new(Arc::new(deno_fs::RealFs)).resolve(specifier, ResolutionMode::Execution); + let data = maybe_sloppy_resolution.as_ref().map(|res| { json!({ "specifier": specifier, - "to": sloppy_resolution.as_specifier(), - "message": message, + "to": res.as_specifier(), + "message": res.as_quick_fix_message(), }) }); - (lsp::DiagnosticSeverity::ERROR, no_local_message(specifier, sloppy_resolution), data) + (lsp::DiagnosticSeverity::ERROR, no_local_message(specifier, maybe_sloppy_resolution.as_ref()), data) }, Self::Redirect { from, to} => (lsp::DiagnosticSeverity::INFORMATION, format!("The import of \"{from}\" was redirected to \"{to}\"."), Some(json!({ "specifier": from, "redirect": to }))), Self::ResolutionError(err) => { diff --git a/cli/lsp/resolver.rs b/cli/lsp/resolver.rs index 7758bbd7cdda5a..bdfd5fd3ee6d8b 100644 --- a/cli/lsp/resolver.rs +++ b/cli/lsp/resolver.rs @@ -21,7 +21,6 @@ use crate::resolver::CjsResolutionStore; use crate::resolver::CliGraphResolver; use crate::resolver::CliGraphResolverOptions; use crate::resolver::CliNodeResolver; -use crate::resolver::SloppyImportsResolver; use crate::resolver::WorkerCliNpmGraphResolver; use crate::util::progress_bar::ProgressBar; use crate::util::progress_bar::ProgressBarStyle; @@ -514,8 +513,6 @@ fn create_graph_resolver( node_resolver: Option<&Arc>, ) -> Arc { let workspace = config_data.map(|d| &d.member_dir.workspace); - let unstable_sloppy_imports = - workspace.is_some_and(|dir| dir.has_unstable("sloppy-imports")); Arc::new(CliGraphResolver::new(CliGraphResolverOptions { node_resolver: node_resolver.cloned(), npm_resolver: npm_resolver.cloned(), @@ -536,9 +533,8 @@ fn create_graph_resolver( maybe_vendor_dir: config_data.and_then(|d| d.vendor_dir.as_ref()), bare_node_builtins_enabled: workspace .is_some_and(|workspace| workspace.has_unstable("bare-node-builtins")), - sloppy_imports_resolver: unstable_sloppy_imports.then(|| { - SloppyImportsResolver::new_without_stat_cache(Arc::new(deno_fs::RealFs)) - }), + sloppy_imports_resolver: config_data + .and_then(|d| d.sloppy_imports_resolver.clone()), })) } diff --git a/cli/resolver.rs b/cli/resolver.rs index c9940b406c13bf..5296b42b8dee0c 100644 --- a/cli/resolver.rs +++ b/cli/resolver.rs @@ -47,21 +47,11 @@ use std::sync::Arc; use crate::args::JsxImportSourceConfig; use crate::args::DENO_DISABLE_PEDANTIC_NODE_WARNINGS; -use crate::colors; use crate::node::CliNodeCodeTranslator; use crate::npm::CliNpmResolver; use crate::npm::InnerCliNpmResolverRef; use crate::util::sync::AtomicFlag; -pub fn format_range_with_colors(range: &deno_graph::Range) -> String { - format!( - "{}:{}:{}", - colors::cyan(range.specifier.as_str()), - colors::yellow(&(range.start.line + 1).to_string()), - colors::yellow(&(range.start.character + 1).to_string()) - ) -} - pub struct ModuleCodeStringSource { pub code: ModuleSourceCode, pub found_url: ModuleSpecifier, @@ -268,8 +258,8 @@ impl CliNodeResolver { pub fn handle_if_in_node_modules( &self, - specifier: ModuleSpecifier, - ) -> Result { + specifier: &ModuleSpecifier, + ) -> Result, AnyError> { // skip canonicalizing if we definitely know it's unnecessary if specifier.scheme() == "file" && specifier.path().contains("/node_modules/") @@ -279,18 +269,18 @@ impl CliNodeResolver { // If so, check if we need to store this specifier as being a CJS // resolution. let specifier = - crate::node::resolve_specifier_into_node_modules(&specifier); + crate::node::resolve_specifier_into_node_modules(specifier); if self.in_npm_package(&specifier) { let resolution = self.node_resolver.url_to_node_resolution(specifier)?; if let NodeResolution::CommonJs(specifier) = &resolution { self.cjs_resolutions.insert(specifier.clone()); } - return Ok(resolution.into_url()); + return Ok(Some(resolution.into_url())); } } - Ok(specifier) + Ok(None) } pub fn url_to_node_resolution( @@ -436,7 +426,7 @@ impl CjsResolutionStore { pub struct CliGraphResolver { node_resolver: Option>, npm_resolver: Option>, - sloppy_imports_resolver: Option, + sloppy_imports_resolver: Option>, workspace_resolver: Arc, maybe_default_jsx_import_source: Option, maybe_default_jsx_import_source_types: Option, @@ -449,7 +439,7 @@ pub struct CliGraphResolver { pub struct CliGraphResolverOptions<'a> { pub node_resolver: Option>, pub npm_resolver: Option>, - pub sloppy_imports_resolver: Option, + pub sloppy_imports_resolver: Option>, pub workspace_resolver: Arc, pub bare_node_builtins_enabled: bool, pub maybe_jsx_import_source_config: Option, @@ -552,12 +542,12 @@ impl Resolver for CliGraphResolver { | MappedResolution::ImportMap(specifier) => { // do sloppy imports resolution if enabled if let Some(sloppy_imports_resolver) = &self.sloppy_imports_resolver { - Ok(sloppy_imports_resolve( - sloppy_imports_resolver, - specifier, - referrer_range, - mode, - )) + Ok( + sloppy_imports_resolver + .resolve(&specifier, mode) + .map(|s| s.into_specifier()) + .unwrap_or(specifier), + ) } else { Ok(specifier) } @@ -681,7 +671,10 @@ impl Resolver for CliGraphResolver { } } - Ok(node_resolver.handle_if_in_node_modules(specifier)?) + Ok(match node_resolver.handle_if_in_node_modules(&specifier)? { + Some(specifier) => specifier, + None => specifier, + }) } Err(err) => { // If byonm, check if the bare specifier resolves to an npm package @@ -700,65 +693,6 @@ impl Resolver for CliGraphResolver { } } -fn sloppy_imports_resolve( - resolver: &SloppyImportsResolver, - specifier: ModuleSpecifier, - referrer_range: &deno_graph::Range, - mode: ResolutionMode, -) -> ModuleSpecifier { - let resolution = resolver.resolve(&specifier, mode); - if mode.is_types() { - // don't bother warning for types resolution because - // we already probably warned during execution resolution - match resolution { - SloppyImportsResolution::None(_) => return specifier, // avoid a clone - _ => return resolution.into_specifier().into_owned(), - } - } - - let hint_message = match &resolution { - SloppyImportsResolution::JsToTs(to_specifier) => { - let to_media_type = MediaType::from_specifier(to_specifier); - let from_media_type = MediaType::from_specifier(&specifier); - format!( - "update {} extension to {}", - from_media_type.as_ts_extension(), - to_media_type.as_ts_extension() - ) - } - SloppyImportsResolution::NoExtension(to_specifier) => { - let to_media_type = MediaType::from_specifier(to_specifier); - format!("add {} extension", to_media_type.as_ts_extension()) - } - SloppyImportsResolution::Directory(to_specifier) => { - let file_name = to_specifier - .path() - .rsplit_once('/') - .map(|(_, file_name)| file_name) - .unwrap_or(to_specifier.path()); - format!("specify path to {} file in directory instead", file_name) - } - SloppyImportsResolution::None(_) => return specifier, - }; - // show a warning when this happens in order to drive - // the user towards correcting these specifiers - if !*DENO_DISABLE_PEDANTIC_NODE_WARNINGS { - log::warn!( - "{} Sloppy module resolution {}\n at {}", - crate::colors::yellow("Warning"), - crate::colors::gray(format!("(hint: {})", hint_message)).to_string(), - if referrer_range.end == deno_graph::Position::zeroed() { - // not worth showing the range in this case - crate::colors::cyan(referrer_range.specifier.as_str()).to_string() - } else { - format_range_with_colors(referrer_range) - }, - ); - } - - resolution.into_specifier().into_owned() -} - #[derive(Debug)] pub struct WorkerCliNpmGraphResolver<'a> { npm_resolver: Option<&'a Arc>, @@ -902,10 +836,8 @@ impl SloppyImportsFsEntry { } } -#[derive(Debug, PartialEq, Eq)] -pub enum SloppyImportsResolution<'a> { - /// No sloppy resolution was found. - None(&'a ModuleSpecifier), +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SloppyImportsResolution { /// Ex. `./file.js` to `./file.ts` JsToTs(ModuleSpecifier), /// Ex. `./file` to `./file.ts` @@ -914,55 +846,46 @@ pub enum SloppyImportsResolution<'a> { Directory(ModuleSpecifier), } -impl<'a> SloppyImportsResolution<'a> { +impl SloppyImportsResolution { pub fn as_specifier(&self) -> &ModuleSpecifier { match self { - Self::None(specifier) => specifier, Self::JsToTs(specifier) => specifier, Self::NoExtension(specifier) => specifier, Self::Directory(specifier) => specifier, } } - pub fn into_specifier(self) -> Cow<'a, ModuleSpecifier> { + pub fn into_specifier(self) -> ModuleSpecifier { match self { - Self::None(specifier) => Cow::Borrowed(specifier), - Self::JsToTs(specifier) => Cow::Owned(specifier), - Self::NoExtension(specifier) => Cow::Owned(specifier), - Self::Directory(specifier) => Cow::Owned(specifier), + Self::JsToTs(specifier) => specifier, + Self::NoExtension(specifier) => specifier, + Self::Directory(specifier) => specifier, } } - pub fn as_suggestion_message(&self) -> Option { - Some(format!("Maybe {}", self.as_base_message()?)) + pub fn as_suggestion_message(&self) -> String { + format!("Maybe {}", self.as_base_message()) } - pub fn as_lsp_quick_fix_message(&self) -> Option { - let message = self.as_base_message()?; + pub fn as_quick_fix_message(&self) -> String { + let message = self.as_base_message(); let mut chars = message.chars(); - Some(format!( + format!( "{}{}.", chars.next().unwrap().to_uppercase(), chars.as_str() - )) + ) } - fn as_base_message(&self) -> Option { + fn as_base_message(&self) -> String { match self { - SloppyImportsResolution::None(_) => None, SloppyImportsResolution::JsToTs(specifier) => { let media_type = MediaType::from_specifier(specifier); - Some(format!( - "change the extension to '{}'", - media_type.as_ts_extension() - )) + format!("change the extension to '{}'", media_type.as_ts_extension()) } SloppyImportsResolution::NoExtension(specifier) => { let media_type = MediaType::from_specifier(specifier); - Some(format!( - "add a '{}' extension", - media_type.as_ts_extension() - )) + format!("add a '{}' extension", media_type.as_ts_extension()) } SloppyImportsResolution::Directory(specifier) => { let file_name = specifier @@ -970,10 +893,7 @@ impl<'a> SloppyImportsResolution<'a> { .rsplit_once('/') .map(|(_, file_name)| file_name) .unwrap_or(specifier.path()); - Some(format!( - "specify path to '{}' file in directory instead", - file_name - )) + format!("specify path to '{}' file in directory instead", file_name) } } } @@ -997,11 +917,11 @@ impl SloppyImportsResolver { Self { fs, cache: None } } - pub fn resolve<'a>( + pub fn resolve( &self, - specifier: &'a ModuleSpecifier, + specifier: &ModuleSpecifier, mode: ResolutionMode, - ) -> SloppyImportsResolution<'a> { + ) -> Option { fn path_without_ext( path: &Path, media_type: MediaType, @@ -1017,11 +937,13 @@ impl SloppyImportsResolver { fn media_types_to_paths( path_no_ext: &str, + original_media_type: MediaType, probe_media_type_types: Vec, reason: SloppyImportsResolutionReason, ) -> Vec<(PathBuf, SloppyImportsResolutionReason)> { probe_media_type_types .into_iter() + .filter(|media_type| *media_type != original_media_type) .map(|media_type| { ( PathBuf::from(format!( @@ -1036,12 +958,10 @@ impl SloppyImportsResolver { } if specifier.scheme() != "file" { - return SloppyImportsResolution::None(specifier); + return None; } - let Ok(path) = specifier_to_file_path(specifier) else { - return SloppyImportsResolution::None(specifier); - }; + let path = specifier_to_file_path(specifier).ok()?; #[derive(Clone, Copy)] enum SloppyImportsResolutionReason { @@ -1066,18 +986,17 @@ impl SloppyImportsResolver { MediaType::Cjs => { vec![MediaType::Dcts, MediaType::Dts, MediaType::Cjs] } - _ => return SloppyImportsResolution::None(specifier), - }; - let Some(path_no_ext) = path_without_ext(&path, media_type) else { - return SloppyImportsResolution::None(specifier); + _ => return None, }; + let path_no_ext = path_without_ext(&path, media_type)?; media_types_to_paths( &path_no_ext, + media_type, probe_media_type_types, SloppyImportsResolutionReason::JsToTs, ) } else { - return SloppyImportsResolution::None(specifier); + return None; } } entry @ None | entry @ Some(SloppyImportsFsEntry::Dir) => { @@ -1121,7 +1040,7 @@ impl SloppyImportsResolver { | MediaType::Wasm | MediaType::TsBuildInfo | MediaType::SourceMap => { - return SloppyImportsResolution::None(specifier) + return None; } MediaType::Unknown => ( if mode.is_types() { @@ -1152,6 +1071,7 @@ impl SloppyImportsResolver { let mut probe_paths = match path_without_ext(&path, media_type) { Some(path_no_ext) => media_types_to_paths( &path_no_ext, + media_type, probe_media_type_types.0, probe_media_type_types.1, ), @@ -1222,7 +1142,7 @@ impl SloppyImportsResolver { } } if probe_paths.is_empty() { - return SloppyImportsResolution::None(specifier); + return None; } probe_paths } @@ -1233,20 +1153,20 @@ impl SloppyImportsResolver { if let Ok(specifier) = ModuleSpecifier::from_file_path(probe_path) { match reason { SloppyImportsResolutionReason::JsToTs => { - return SloppyImportsResolution::JsToTs(specifier) + return Some(SloppyImportsResolution::JsToTs(specifier)); } SloppyImportsResolutionReason::NoExtension => { - return SloppyImportsResolution::NoExtension(specifier) + return Some(SloppyImportsResolution::NoExtension(specifier)); } SloppyImportsResolutionReason::Directory => { - return SloppyImportsResolution::Directory(specifier) + return Some(SloppyImportsResolution::Directory(specifier)); } } } } } - SloppyImportsResolution::None(specifier) + None } fn stat_sync(&self, path: &Path) -> Option { @@ -1276,9 +1196,22 @@ mod test { #[test] fn test_unstable_sloppy_imports() { - fn resolve(specifier: &ModuleSpecifier) -> SloppyImportsResolution { + fn resolve(specifier: &ModuleSpecifier) -> Option { + resolve_with_mode(specifier, ResolutionMode::Execution) + } + + fn resolve_types( + specifier: &ModuleSpecifier, + ) -> Option { + resolve_with_mode(specifier, ResolutionMode::Types) + } + + fn resolve_with_mode( + specifier: &ModuleSpecifier, + mode: ResolutionMode, + ) -> Option { SloppyImportsResolver::new(Arc::new(deno_fs::RealFs)) - .resolve(specifier, ResolutionMode::Execution) + .resolve(specifier, mode) } let context = TestContext::default(); @@ -1288,11 +1221,7 @@ mod test { for (ext_from, ext_to) in [("js", "ts"), ("js", "tsx"), ("mjs", "mts")] { let ts_file = temp_dir.join(format!("file.{}", ext_to)); ts_file.write(""); - let ts_file_uri = ts_file.uri_file(); - assert_eq!( - resolve(&ts_file.uri_file()), - SloppyImportsResolution::None(&ts_file_uri), - ); + assert_eq!(resolve(&ts_file.uri_file()), None); assert_eq!( resolve( &temp_dir @@ -1300,7 +1229,7 @@ mod test { .join(&format!("file.{}", ext_from)) .unwrap() ), - SloppyImportsResolution::JsToTs(ts_file.uri_file()), + Some(SloppyImportsResolution::JsToTs(ts_file.uri_file())), ); ts_file.remove_file(); } @@ -1316,7 +1245,7 @@ mod test { .join("file") // no ext .unwrap() ), - SloppyImportsResolution::NoExtension(file.uri_file()), + Some(SloppyImportsResolution::NoExtension(file.uri_file())) ); file.remove_file(); } @@ -1327,11 +1256,15 @@ mod test { ts_file.write(""); let js_file = temp_dir.join("file.js"); js_file.write(""); - let js_file_uri = js_file.uri_file(); - assert_eq!( - resolve(&js_file.uri_file()), - SloppyImportsResolution::None(&js_file_uri), - ); + assert_eq!(resolve(&js_file.uri_file()), None); + } + + // only js exists, .js specified + { + let js_only_file = temp_dir.join("js_only.js"); + js_only_file.write(""); + assert_eq!(resolve(&js_only_file.uri_file()), None); + assert_eq!(resolve_types(&js_only_file.uri_file()), None); } // resolving a directory to an index file @@ -1342,7 +1275,7 @@ mod test { index_file.write(""); assert_eq!( resolve(&routes_dir.uri_file()), - SloppyImportsResolution::Directory(index_file.uri_file()), + Some(SloppyImportsResolution::Directory(index_file.uri_file())), ); } @@ -1356,26 +1289,19 @@ mod test { api_file.write(""); assert_eq!( resolve(&api_dir.uri_file()), - SloppyImportsResolution::NoExtension(api_file.uri_file()), + Some(SloppyImportsResolution::NoExtension(api_file.uri_file())), ); } } #[test] fn test_sloppy_import_resolution_suggestion_message() { - // none - let url = ModuleSpecifier::parse("file:///dir/index.js").unwrap(); - assert_eq!( - SloppyImportsResolution::None(&url).as_suggestion_message(), - None, - ); // directory assert_eq!( SloppyImportsResolution::Directory( ModuleSpecifier::parse("file:///dir/index.js").unwrap() ) - .as_suggestion_message() - .unwrap(), + .as_suggestion_message(), "Maybe specify path to 'index.js' file in directory instead" ); // no ext @@ -1383,8 +1309,7 @@ mod test { SloppyImportsResolution::NoExtension( ModuleSpecifier::parse("file:///dir/index.mjs").unwrap() ) - .as_suggestion_message() - .unwrap(), + .as_suggestion_message(), "Maybe add a '.mjs' extension" ); // js to ts @@ -1392,8 +1317,7 @@ mod test { SloppyImportsResolution::JsToTs( ModuleSpecifier::parse("file:///dir/index.mts").unwrap() ) - .as_suggestion_message() - .unwrap(), + .as_suggestion_message(), "Maybe change the extension to '.mts'" ); } diff --git a/cli/standalone/mod.rs b/cli/standalone/mod.rs index 561c99c983a50b..c91f3bec9086e6 100644 --- a/cli/standalone/mod.rs +++ b/cli/standalone/mod.rs @@ -241,10 +241,13 @@ impl ModuleLoader for EmbeddedModuleLoader { } } - self - .shared - .node_resolver - .handle_if_in_node_modules(specifier) + Ok( + self + .shared + .node_resolver + .handle_if_in_node_modules(&specifier)? + .unwrap_or(specifier), + ) } Err(err) if err.is_unmapped_bare_specifier() && referrer.scheme() == "file" => diff --git a/cli/tools/lint/linter.rs b/cli/tools/lint/linter.rs new file mode 100644 index 00000000000000..f6ea76c14c987e --- /dev/null +++ b/cli/tools/lint/linter.rs @@ -0,0 +1,242 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use std::path::Path; + +use deno_ast::MediaType; +use deno_ast::ModuleSpecifier; +use deno_ast::ParsedSource; +use deno_ast::SourceTextInfo; +use deno_core::anyhow::Context; +use deno_core::error::AnyError; +use deno_graph::ModuleGraph; +use deno_lint::diagnostic::LintDiagnostic; +use deno_lint::linter::LintConfig as DenoLintConfig; +use deno_lint::linter::LintFileOptions; +use deno_lint::linter::Linter as DenoLintLinter; +use deno_lint::linter::LinterOptions; + +use crate::util::fs::atomic_write_file_with_retries; +use crate::util::fs::specifier_from_file_path; + +use super::rules::FileOrPackageLintRule; +use super::rules::PackageLintRule; +use super::ConfiguredRules; + +pub struct CliLinterOptions { + pub configured_rules: ConfiguredRules, + pub fix: bool, + pub deno_lint_config: DenoLintConfig, +} + +#[derive(Debug)] +pub struct CliLinter { + fix: bool, + package_rules: Vec>, + linter: DenoLintLinter, + deno_lint_config: DenoLintConfig, +} + +impl CliLinter { + pub fn new(options: CliLinterOptions) -> Self { + let rules = options.configured_rules.rules; + let mut deno_lint_rules = Vec::with_capacity(rules.len()); + let mut package_rules = Vec::with_capacity(rules.len()); + for rule in rules { + match rule.into_file_or_pkg_rule() { + FileOrPackageLintRule::File(rule) => { + deno_lint_rules.push(rule); + } + FileOrPackageLintRule::Package(rule) => { + package_rules.push(rule); + } + } + } + Self { + fix: options.fix, + package_rules, + linter: DenoLintLinter::new(LinterOptions { + rules: deno_lint_rules, + all_rule_codes: options.configured_rules.all_rule_codes, + custom_ignore_file_directive: None, + custom_ignore_diagnostic_directive: None, + }), + deno_lint_config: options.deno_lint_config, + } + } + + pub fn has_package_rules(&self) -> bool { + !self.package_rules.is_empty() + } + + pub fn lint_package( + &self, + graph: &ModuleGraph, + entrypoints: &[ModuleSpecifier], + ) -> Vec { + let mut diagnostics = Vec::new(); + for rule in &self.package_rules { + diagnostics.extend(rule.lint_package(graph, entrypoints)); + } + diagnostics + } + + pub fn lint_with_ast( + &self, + parsed_source: &ParsedSource, + ) -> Vec { + self + .linter + .lint_with_ast(parsed_source, self.deno_lint_config.clone()) + } + + pub fn lint_file( + &self, + file_path: &Path, + source_code: String, + ) -> Result<(ParsedSource, Vec), AnyError> { + let specifier = specifier_from_file_path(file_path)?; + let media_type = MediaType::from_specifier(&specifier); + + if self.fix { + self.lint_file_and_fix(&specifier, media_type, source_code, file_path) + } else { + self + .linter + .lint_file(LintFileOptions { + specifier, + media_type, + source_code, + config: self.deno_lint_config.clone(), + }) + .map_err(AnyError::from) + } + } + + fn lint_file_and_fix( + &self, + specifier: &ModuleSpecifier, + media_type: MediaType, + source_code: String, + file_path: &Path, + ) -> Result<(ParsedSource, Vec), deno_core::anyhow::Error> { + // initial lint + let (source, diagnostics) = self.linter.lint_file(LintFileOptions { + specifier: specifier.clone(), + media_type, + source_code, + config: self.deno_lint_config.clone(), + })?; + + // Try applying fixes repeatedly until the file has none left or + // a maximum number of iterations is reached. This is necessary + // because lint fixes may overlap and so we can't always apply + // them in one pass. + let mut source = source; + let mut diagnostics = diagnostics; + let mut fix_iterations = 0; + loop { + let change = apply_lint_fixes_and_relint( + specifier, + media_type, + &self.linter, + self.deno_lint_config.clone(), + source.text_info_lazy(), + &diagnostics, + )?; + match change { + Some(change) => { + source = change.0; + diagnostics = change.1; + } + None => { + break; + } + } + fix_iterations += 1; + if fix_iterations > 5 { + log::warn!( + concat!( + "Reached maximum number of fix iterations for '{}'. There's ", + "probably a bug in Deno. Please fix this file manually.", + ), + specifier, + ); + break; + } + } + + if fix_iterations > 0 { + // everything looks good and the file still parses, so write it out + atomic_write_file_with_retries( + file_path, + source.text().as_ref(), + crate::cache::CACHE_PERM, + ) + .context("Failed writing fix to file.")?; + } + + Ok((source, diagnostics)) + } +} + +fn apply_lint_fixes_and_relint( + specifier: &ModuleSpecifier, + media_type: MediaType, + linter: &DenoLintLinter, + config: DenoLintConfig, + text_info: &SourceTextInfo, + diagnostics: &[LintDiagnostic], +) -> Result)>, AnyError> { + let Some(new_text) = apply_lint_fixes(text_info, diagnostics) else { + return Ok(None); + }; + linter + .lint_file(LintFileOptions { + specifier: specifier.clone(), + source_code: new_text, + media_type, + config, + }) + .map(Some) + .context( + "An applied lint fix caused a syntax error. Please report this bug.", + ) +} + +fn apply_lint_fixes( + text_info: &SourceTextInfo, + diagnostics: &[LintDiagnostic], +) -> Option { + if diagnostics.is_empty() { + return None; + } + + let file_start = text_info.range().start; + let mut quick_fixes = diagnostics + .iter() + // use the first quick fix + .filter_map(|d| d.details.fixes.first()) + .flat_map(|fix| fix.changes.iter()) + .map(|change| deno_ast::TextChange { + range: change.range.as_byte_range(file_start), + new_text: change.new_text.to_string(), + }) + .collect::>(); + if quick_fixes.is_empty() { + return None; + } + // remove any overlapping text changes, we'll circle + // back for another pass to fix the remaining + quick_fixes.sort_by_key(|change| change.range.start); + for i in (1..quick_fixes.len()).rev() { + let cur = &quick_fixes[i]; + let previous = &quick_fixes[i - 1]; + let is_overlapping = cur.range.start < previous.range.end; + if is_overlapping { + quick_fixes.remove(i); + } + } + let new_text = + deno_ast::apply_text_changes(text_info.text_str(), quick_fixes); + Some(new_text) +} diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index 9421291209e320..e01b384307057f 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -3,18 +3,13 @@ //! This module provides file linting utilities using //! [`deno_lint`](https://github.com/denoland/deno_lint). use deno_ast::diagnostics::Diagnostic; -use deno_ast::MediaType; use deno_ast::ModuleSpecifier; use deno_ast::ParsedSource; -use deno_ast::SourceRange; -use deno_ast::SourceTextInfo; -use deno_config::deno_json::ConfigFile; +use deno_config::deno_json::LintRulesConfig; use deno_config::glob::FileCollector; use deno_config::glob::FilePatterns; use deno_config::workspace::WorkspaceDirectory; use deno_core::anyhow::anyhow; -use deno_core::anyhow::bail; -use deno_core::anyhow::Context; use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::futures::future::LocalBoxFuture; @@ -23,19 +18,12 @@ use deno_core::parking_lot::Mutex; use deno_core::serde_json; use deno_core::unsync::future::LocalFutureExt; use deno_core::unsync::future::SharedLocal; -use deno_graph::FastCheckDiagnostic; use deno_graph::ModuleGraph; use deno_lint::diagnostic::LintDiagnostic; use deno_lint::linter::LintConfig; -use deno_lint::linter::LintFileOptions; -use deno_lint::linter::Linter; -use deno_lint::linter::LinterBuilder; -use deno_lint::rules; -use deno_lint::rules::LintRule; use log::debug; use log::info; use serde::Serialize; -use std::borrow::Cow; use std::collections::HashSet; use std::fs; use std::io::stdin; @@ -50,7 +38,6 @@ use crate::args::Flags; use crate::args::LintFlags; use crate::args::LintOptions; use crate::args::LintReporterKind; -use crate::args::LintRulesConfig; use crate::args::WorkspaceLintOptions; use crate::cache::Caches; use crate::cache::IncrementalCache; @@ -60,11 +47,17 @@ use crate::graph_util::ModuleGraphCreator; use crate::tools::fmt::run_parallelized; use crate::util::file_watcher; use crate::util::fs::canonicalize_path; -use crate::util::fs::specifier_from_file_path; use crate::util::path::is_script_ext; use crate::util::sync::AtomicFlag; -pub mod no_slow_types; +mod linter; +mod rules; + +pub use linter::CliLinter; +pub use linter::CliLinterOptions; +pub use rules::collect_no_slow_type_diagnostics; +pub use rules::ConfiguredRules; +pub use rules::LintRuleProvider; static STDIN_FILE_NAME: &str = "$deno$stdin.ts"; @@ -120,6 +113,7 @@ pub async fn lint( let mut linter = WorkspaceLinter::new( factory.caches()?.clone(), + factory.lint_rule_provider().await?, factory.module_graph_creator().await?.clone(), cli_options.start_dir.clone(), &cli_options.resolve_workspace_lint_options(&lint_flags)?, @@ -157,12 +151,15 @@ pub async fn lint( let lint_config = start_dir .to_lint_config(FilePatterns::new_with_base(start_dir.dir_path()))?; let lint_options = LintOptions::resolve(lint_config, &lint_flags); - let lint_rules = get_config_rules_err_empty( - lint_options.rules, - start_dir.maybe_deno_json().map(|c| c.as_ref()), - )?; + let lint_rules = factory + .lint_rule_provider() + .await? + .resolve_lint_rules_err_empty( + lint_options.rules, + start_dir.maybe_deno_json().map(|c| c.as_ref()), + )?; let file_path = cli_options.initial_cwd().join(STDIN_FILE_NAME); - let r = lint_stdin(&file_path, lint_rules.rules, deno_lint_config); + let r = lint_stdin(&file_path, lint_rules, deno_lint_config); let success = handle_lint_result( &file_path.to_string_lossy(), r, @@ -173,6 +170,7 @@ pub async fn lint( } else { let mut linter = WorkspaceLinter::new( factory.caches()?.clone(), + factory.lint_rule_provider().await?, factory.module_graph_creator().await?.clone(), cli_options.start_dir.clone(), &workspace_lint_options, @@ -234,6 +232,7 @@ type WorkspaceModuleGraphFuture = struct WorkspaceLinter { caches: Arc, + lint_rule_provider: LintRuleProvider, module_graph_creator: Arc, workspace_dir: Arc, reporter_lock: Arc>>, @@ -245,6 +244,7 @@ struct WorkspaceLinter { impl WorkspaceLinter { pub fn new( caches: Arc, + lint_rule_provider: LintRuleProvider, module_graph_creator: Arc, workspace_dir: Arc, workspace_options: &WorkspaceLintOptions, @@ -253,6 +253,7 @@ impl WorkspaceLinter { Arc::new(Mutex::new(create_reporter(workspace_options.reporter_kind))); Self { caches, + lint_rule_provider, module_graph_creator, workspace_dir, reporter_lock, @@ -271,18 +272,27 @@ impl WorkspaceLinter { ) -> Result<(), AnyError> { self.file_count += paths.len(); - let lint_rules = get_config_rules_err_empty( + let lint_rules = self.lint_rule_provider.resolve_lint_rules_err_empty( lint_options.rules, member_dir.maybe_deno_json().map(|c| c.as_ref()), )?; - let incremental_cache = Arc::new(IncrementalCache::new( - self.caches.lint_incremental_cache_db(), - &lint_rules.incremental_cache_state(), - &paths, - )); + let maybe_incremental_cache = + lint_rules.incremental_cache_state().map(|state| { + Arc::new(IncrementalCache::new( + self.caches.lint_incremental_cache_db(), + &state, + &paths, + )) + }); + + let linter = Arc::new(CliLinter::new(CliLinterOptions { + configured_rules: lint_rules, + fix: lint_options.fix, + deno_lint_config: lint_config, + })); let mut futures = Vec::with_capacity(2); - if lint_rules.no_slow_types { + if linter.has_package_rules() { if self.workspace_module_graph.is_none() { let module_graph_creator = self.module_graph_creator.clone(); let packages = self.workspace_dir.jsr_packages_for_publish(); @@ -304,6 +314,7 @@ impl WorkspaceLinter { if let Some(publish_config) = publish_config { let has_error = self.has_error.clone(); let reporter_lock = self.reporter_lock.clone(); + let linter = linter.clone(); let path_urls = paths .iter() .filter_map(|p| ModuleSpecifier::from_file_path(p).ok()) @@ -318,16 +329,12 @@ impl WorkspaceLinter { if !export_urls.iter().any(|url| path_urls.contains(url)) { return Ok(()); // entrypoint is not specified, so skip } - let diagnostics = no_slow_types::collect_no_slow_type_diagnostics( - &export_urls, - &graph, - ); + let diagnostics = linter.lint_package(&graph, &export_urls); if !diagnostics.is_empty() { has_error.raise(); let mut reporter = reporter_lock.lock(); for diagnostic in &diagnostics { - reporter - .visit_diagnostic(LintOrCliDiagnostic::FastCheck(diagnostic)); + reporter.visit_diagnostic(diagnostic); } } Ok(()) @@ -339,11 +346,9 @@ impl WorkspaceLinter { futures.push({ let has_error = self.has_error.clone(); - let linter = create_linter(lint_rules.rules); let reporter_lock = self.reporter_lock.clone(); - let incremental_cache = incremental_cache.clone(); - let lint_config = lint_config.clone(); - let fix = lint_options.fix; + let maybe_incremental_cache = maybe_incremental_cache.clone(); + let linter = linter.clone(); async move { run_parallelized(paths, { move |file_path| { @@ -351,19 +356,23 @@ impl WorkspaceLinter { deno_ast::strip_bom(fs::read_to_string(&file_path)?); // don't bother rechecking this file if it didn't have any diagnostics before - if incremental_cache.is_file_same(&file_path, &file_text) { - return Ok(()); + if let Some(incremental_cache) = &maybe_incremental_cache { + if incremental_cache.is_file_same(&file_path, &file_text) { + return Ok(()); + } } - let r = lint_file(&linter, &file_path, file_text, lint_config, fix); + let r = linter.lint_file(&file_path, file_text); if let Ok((file_source, file_diagnostics)) = &r { - if file_diagnostics.is_empty() { - // update the incremental cache if there were no diagnostics - incremental_cache.update_file( - &file_path, - // ensure the returned text is used here as it may have been modified via --fix - file_source.text(), - ) + if let Some(incremental_cache) = &maybe_incremental_cache { + if file_diagnostics.is_empty() { + // update the incremental cache if there were no diagnostics + incremental_cache.update_file( + &file_path, + // ensure the returned text is used here as it may have been modified via --fix + file_source.text(), + ) + } } } @@ -384,9 +393,21 @@ impl WorkspaceLinter { .boxed_local() }); - deno_core::futures::future::try_join_all(futures).await?; + if lint_options.fix { + // run sequentially when using `--fix` to lower the chances of weird + // bugs where a file level fix affects a package level diagnostic though + // it probably will happen anyway + for future in futures { + future.await?; + } + } else { + deno_core::futures::future::try_join_all(futures).await?; + } + + if let Some(incremental_cache) = &maybe_incremental_cache { + incremental_cache.wait_completion().await; + } - incremental_cache.wait_completion().await; Ok(()) } @@ -410,11 +431,17 @@ fn collect_lint_files( #[allow(clippy::print_stdout)] pub fn print_rules_list(json: bool, maybe_rules_tags: Option>) { - let lint_rules = if maybe_rules_tags.is_none() { - rules::get_all_rules() - } else { - rules::get_filtered_rules(maybe_rules_tags, None, None) - }; + let rule_provider = LintRuleProvider::new(None, None); + let lint_rules = rule_provider + .resolve_lint_rules( + LintRulesConfig { + tags: maybe_rules_tags.clone(), + include: None, + exclude: None, + }, + None, + ) + .rules; if json { let json_rules: Vec = lint_rules @@ -442,186 +469,19 @@ pub fn print_rules_list(json: bool, maybe_rules_tags: Option>) { } println!( "{}", - colors::gray(format!( - " help: https://lint.deno.land/#{}", - rule.code() - )) + colors::gray(format!(" help: {}", rule.help_docs_url())) ); println!(); } } } -pub fn create_linter(rules: Vec<&'static dyn LintRule>) -> Linter { - LinterBuilder::default() - .ignore_file_directive("deno-lint-ignore-file") - .ignore_diagnostic_directive("deno-lint-ignore") - .rules(rules) - .build() -} - -fn lint_file( - linter: &Linter, - file_path: &Path, - source_code: String, - config: LintConfig, - fix: bool, -) -> Result<(ParsedSource, Vec), AnyError> { - let specifier = specifier_from_file_path(file_path)?; - let media_type = MediaType::from_specifier(&specifier); - - if fix { - lint_file_and_fix( - linter, - &specifier, - media_type, - source_code, - file_path, - config, - ) - } else { - linter - .lint_file(LintFileOptions { - specifier, - media_type, - source_code, - config, - }) - .map_err(AnyError::from) - } -} - -fn lint_file_and_fix( - linter: &Linter, - specifier: &ModuleSpecifier, - media_type: MediaType, - source_code: String, - file_path: &Path, - config: LintConfig, -) -> Result<(ParsedSource, Vec), deno_core::anyhow::Error> { - // initial lint - let (source, diagnostics) = linter.lint_file(LintFileOptions { - specifier: specifier.clone(), - media_type, - source_code, - config: config.clone(), - })?; - - // Try applying fixes repeatedly until the file has none left or - // a maximum number of iterations is reached. This is necessary - // because lint fixes may overlap and so we can't always apply - // them in one pass. - let mut source = source; - let mut diagnostics = diagnostics; - let mut fix_iterations = 0; - loop { - let change = apply_lint_fixes_and_relint( - specifier, - media_type, - linter, - config.clone(), - source.text_info_lazy(), - &diagnostics, - )?; - match change { - Some(change) => { - source = change.0; - diagnostics = change.1; - } - None => { - break; - } - } - fix_iterations += 1; - if fix_iterations > 5 { - log::warn!( - concat!( - "Reached maximum number of fix iterations for '{}'. There's ", - "probably a bug in Deno. Please fix this file manually.", - ), - specifier, - ); - break; - } - } - - if fix_iterations > 0 { - // everything looks good and the file still parses, so write it out - fs::write(file_path, source.text().as_ref()) - .context("Failed writing fix to file.")?; - } - - Ok((source, diagnostics)) -} - -fn apply_lint_fixes_and_relint( - specifier: &ModuleSpecifier, - media_type: MediaType, - linter: &Linter, - config: LintConfig, - text_info: &SourceTextInfo, - diagnostics: &[LintDiagnostic], -) -> Result)>, AnyError> { - let Some(new_text) = apply_lint_fixes(text_info, diagnostics) else { - return Ok(None); - }; - linter - .lint_file(LintFileOptions { - specifier: specifier.clone(), - source_code: new_text, - media_type, - config, - }) - .map(Some) - .context( - "An applied lint fix caused a syntax error. Please report this bug.", - ) -} - -fn apply_lint_fixes( - text_info: &SourceTextInfo, - diagnostics: &[LintDiagnostic], -) -> Option { - if diagnostics.is_empty() { - return None; - } - - let file_start = text_info.range().start; - let mut quick_fixes = diagnostics - .iter() - // use the first quick fix - .filter_map(|d| d.fixes.first()) - .flat_map(|fix| fix.changes.iter()) - .map(|change| deno_ast::TextChange { - range: change.range.as_byte_range(file_start), - new_text: change.new_text.to_string(), - }) - .collect::>(); - if quick_fixes.is_empty() { - return None; - } - // remove any overlapping text changes, we'll circle - // back for another pass to fix the remaining - quick_fixes.sort_by_key(|change| change.range.start); - for i in (1..quick_fixes.len()).rev() { - let cur = &quick_fixes[i]; - let previous = &quick_fixes[i - 1]; - let is_overlapping = cur.range.start < previous.range.end; - if is_overlapping { - quick_fixes.remove(i); - } - } - let new_text = - deno_ast::apply_text_changes(text_info.text_str(), quick_fixes); - Some(new_text) -} - /// Lint stdin and write result to stdout. /// Treats input as TypeScript. /// Compatible with `--json` flag. fn lint_stdin( file_path: &Path, - lint_rules: Vec<&'static dyn LintRule>, + configured_rules: ConfiguredRules, deno_lint_config: LintConfig, ) -> Result<(ParsedSource, Vec), AnyError> { let mut source_code = String::new(); @@ -629,15 +489,14 @@ fn lint_stdin( return Err(generic_error("Failed to read from stdin")); } - let linter = create_linter(lint_rules); + let linter = CliLinter::new(CliLinterOptions { + fix: false, + configured_rules, + deno_lint_config, + }); linter - .lint_file(LintFileOptions { - specifier: specifier_from_file_path(file_path)?, - source_code: deno_ast::strip_bom(source_code), - media_type: MediaType::TypeScript, - config: deno_lint_config, - }) + .lint_file(file_path, deno_ast::strip_bom(source_code)) .map_err(AnyError::from) } @@ -656,11 +515,18 @@ fn handle_lint_result( } } file_diagnostics.sort_by(|a, b| match a.specifier.cmp(&b.specifier) { - std::cmp::Ordering::Equal => a.range.start.cmp(&b.range.start), + std::cmp::Ordering::Equal => { + let a_start = a.range.as_ref().map(|r| r.range.start); + let b_start = b.range.as_ref().map(|r| r.range.start); + match a_start.cmp(&b_start) { + std::cmp::Ordering::Equal => a.details.code.cmp(&b.details.code), + other => other, + } + } file_order => file_order, }); for d in &file_diagnostics { - reporter.visit_diagnostic(LintOrCliDiagnostic::Lint(d)); + reporter.visit_diagnostic(d); } file_diagnostics.is_empty() } @@ -671,99 +537,8 @@ fn handle_lint_result( } } -#[derive(Clone, Copy)] -pub enum LintOrCliDiagnostic<'a> { - Lint(&'a LintDiagnostic), - FastCheck(&'a FastCheckDiagnostic), -} - -impl<'a> LintOrCliDiagnostic<'a> { - pub fn specifier(&self) -> &ModuleSpecifier { - match self { - LintOrCliDiagnostic::Lint(d) => &d.specifier, - LintOrCliDiagnostic::FastCheck(d) => d.specifier(), - } - } - - pub fn range(&self) -> Option<(&SourceTextInfo, SourceRange)> { - match self { - LintOrCliDiagnostic::Lint(d) => Some((&d.text_info, d.range)), - LintOrCliDiagnostic::FastCheck(d) => { - d.range().map(|r| (&r.text_info, r.range)) - } - } - } -} - -impl<'a> deno_ast::diagnostics::Diagnostic for LintOrCliDiagnostic<'a> { - fn level(&self) -> deno_ast::diagnostics::DiagnosticLevel { - match self { - LintOrCliDiagnostic::Lint(d) => d.level(), - LintOrCliDiagnostic::FastCheck(d) => d.level(), - } - } - - fn code(&self) -> Cow<'_, str> { - match self { - LintOrCliDiagnostic::Lint(d) => d.code(), - LintOrCliDiagnostic::FastCheck(_) => Cow::Borrowed("no-slow-types"), - } - } - - fn message(&self) -> Cow<'_, str> { - match self { - LintOrCliDiagnostic::Lint(d) => d.message(), - LintOrCliDiagnostic::FastCheck(d) => d.message(), - } - } - - fn location(&self) -> deno_ast::diagnostics::DiagnosticLocation { - match self { - LintOrCliDiagnostic::Lint(d) => d.location(), - LintOrCliDiagnostic::FastCheck(d) => d.location(), - } - } - - fn snippet(&self) -> Option> { - match self { - LintOrCliDiagnostic::Lint(d) => d.snippet(), - LintOrCliDiagnostic::FastCheck(d) => d.snippet(), - } - } - - fn hint(&self) -> Option> { - match self { - LintOrCliDiagnostic::Lint(d) => d.hint(), - LintOrCliDiagnostic::FastCheck(d) => d.hint(), - } - } - - fn snippet_fixed( - &self, - ) -> Option> { - match self { - LintOrCliDiagnostic::Lint(d) => d.snippet_fixed(), - LintOrCliDiagnostic::FastCheck(d) => d.snippet_fixed(), - } - } - - fn info(&self) -> Cow<'_, [Cow<'_, str>]> { - match self { - LintOrCliDiagnostic::Lint(d) => d.info(), - LintOrCliDiagnostic::FastCheck(d) => d.info(), - } - } - - fn docs_url(&self) -> Option> { - match self { - LintOrCliDiagnostic::Lint(d) => d.docs_url(), - LintOrCliDiagnostic::FastCheck(d) => d.docs_url(), - } - } -} - trait LintReporter { - fn visit_diagnostic(&mut self, d: LintOrCliDiagnostic); + fn visit_diagnostic(&mut self, d: &LintDiagnostic); fn visit_error(&mut self, file_path: &str, err: &AnyError); fn close(&mut self, check_count: usize); } @@ -789,12 +564,10 @@ impl PrettyLintReporter { } impl LintReporter for PrettyLintReporter { - fn visit_diagnostic(&mut self, d: LintOrCliDiagnostic) { + fn visit_diagnostic(&mut self, d: &LintDiagnostic) { self.lint_count += 1; - if let LintOrCliDiagnostic::Lint(d) = d { - if !d.fixes.is_empty() { - self.fixable_diagnostics += 1; - } + if !d.details.fixes.is_empty() { + self.fixable_diagnostics += 1; } log::error!("{}\n", d.display()); @@ -838,15 +611,17 @@ impl CompactLintReporter { } impl LintReporter for CompactLintReporter { - fn visit_diagnostic(&mut self, d: LintOrCliDiagnostic) { + fn visit_diagnostic(&mut self, d: &LintDiagnostic) { self.lint_count += 1; - match d.range() { - Some((text_info, range)) => { + match &d.range { + Some(range) => { + let text_info = &range.text_info; + let range = &range.range; let line_and_column = text_info.line_and_column_display(range.start); log::error!( "{}: line {}, col {} - {} ({})", - d.specifier(), + d.specifier, line_and_column.line_number, line_and_column.column_number, d.message(), @@ -854,7 +629,7 @@ impl LintReporter for CompactLintReporter { ) } None => { - log::error!("{}: {} ({})", d.specifier(), d.message(), d.code()) + log::error!("{}: {} ({})", d.specifier, d.message(), d.code()) } } } @@ -932,18 +707,22 @@ impl JsonLintReporter { } impl LintReporter for JsonLintReporter { - fn visit_diagnostic(&mut self, d: LintOrCliDiagnostic) { + fn visit_diagnostic(&mut self, d: &LintDiagnostic) { self.diagnostics.push(JsonLintDiagnostic { - filename: d.specifier().to_string(), - range: d.range().map(|(text_info, range)| JsonLintDiagnosticRange { - start: JsonDiagnosticLintPosition::new( - range.start.as_byte_index(text_info.range().start), - text_info.line_and_column_index(range.start), - ), - end: JsonDiagnosticLintPosition::new( - range.end.as_byte_index(text_info.range().start), - text_info.line_and_column_index(range.end), - ), + filename: d.specifier.to_string(), + range: d.range.as_ref().map(|range| { + let text_info = &range.text_info; + let range = range.range; + JsonLintDiagnosticRange { + start: JsonDiagnosticLintPosition::new( + range.start.as_byte_index(text_info.range().start), + text_info.line_and_column_index(range.start), + ), + end: JsonDiagnosticLintPosition::new( + range.end.as_byte_index(text_info.range().start), + text_info.line_and_column_index(range.end), + ), + } }), message: d.message().to_string(), code: d.code().to_string(), @@ -994,116 +773,3 @@ fn sort_diagnostics(diagnostics: &mut [JsonLintDiagnostic]) { } }); } - -fn get_config_rules_err_empty( - rules: LintRulesConfig, - maybe_config_file: Option<&ConfigFile>, -) -> Result { - let lint_rules = get_configured_rules(rules, maybe_config_file); - if lint_rules.rules.is_empty() { - bail!("No rules have been configured") - } - Ok(lint_rules) -} - -#[derive(Debug, Clone)] -pub struct ConfiguredRules { - pub rules: Vec<&'static dyn LintRule>, - // cli specific rules - pub no_slow_types: bool, -} - -impl Default for ConfiguredRules { - fn default() -> Self { - get_configured_rules(Default::default(), None) - } -} - -impl ConfiguredRules { - fn incremental_cache_state(&self) -> Vec<&str> { - // use a hash of the rule names in order to bust the cache - let mut names = self.rules.iter().map(|r| r.code()).collect::>(); - // ensure this is stable by sorting it - names.sort_unstable(); - if self.no_slow_types { - names.push("no-slow-types"); - } - names - } -} - -pub fn get_configured_rules( - rules: LintRulesConfig, - maybe_config_file: Option<&ConfigFile>, -) -> ConfiguredRules { - const NO_SLOW_TYPES_NAME: &str = "no-slow-types"; - let implicit_no_slow_types = - maybe_config_file.map(|c| c.is_package()).unwrap_or(false); - let no_slow_types = implicit_no_slow_types - && !rules - .exclude - .as_ref() - .map(|exclude| exclude.iter().any(|i| i == NO_SLOW_TYPES_NAME)) - .unwrap_or(false); - let rules = rules::get_filtered_rules( - rules - .tags - .or_else(|| Some(get_default_tags(maybe_config_file))), - rules.exclude.map(|exclude| { - exclude - .into_iter() - .filter(|c| c != NO_SLOW_TYPES_NAME) - .collect() - }), - rules.include.map(|include| { - include - .into_iter() - .filter(|c| c != NO_SLOW_TYPES_NAME) - .collect() - }), - ); - ConfiguredRules { - rules, - no_slow_types, - } -} - -fn get_default_tags(maybe_config_file: Option<&ConfigFile>) -> Vec { - let mut tags = Vec::with_capacity(2); - tags.push("recommended".to_string()); - if maybe_config_file.map(|c| c.is_package()).unwrap_or(false) { - tags.push("jsr".to_string()); - } - tags -} - -#[cfg(test)] -mod test { - use deno_lint::rules::get_recommended_rules; - - use super::*; - use crate::args::LintRulesConfig; - - #[test] - fn recommended_rules_when_no_tags_in_config() { - let rules_config = LintRulesConfig { - exclude: Some(vec!["no-debugger".to_string()]), - include: None, - tags: None, - }; - let rules = get_configured_rules(rules_config, None); - let mut rule_names = rules - .rules - .into_iter() - .map(|r| r.code().to_string()) - .collect::>(); - rule_names.sort(); - let mut recommended_rule_names = get_recommended_rules() - .into_iter() - .map(|r| r.code().to_string()) - .filter(|n| n != "no-debugger") - .collect::>(); - recommended_rule_names.sort(); - assert_eq!(rule_names, recommended_rule_names); - } -} diff --git a/cli/tools/lint/no_slow_types.rs b/cli/tools/lint/no_slow_types.rs deleted file mode 100644 index 6bb108a88d6823..00000000000000 --- a/cli/tools/lint/no_slow_types.rs +++ /dev/null @@ -1,38 +0,0 @@ -// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. - -use deno_ast::diagnostics::Diagnostic; -use deno_ast::ModuleSpecifier; -use deno_graph::FastCheckDiagnostic; -use deno_graph::ModuleGraph; - -/// Collects diagnostics from the module graph for the -/// given package's export URLs. -pub fn collect_no_slow_type_diagnostics( - package_export_urls: &[ModuleSpecifier], - graph: &ModuleGraph, -) -> Vec { - let mut js_exports = package_export_urls - .iter() - .filter_map(|url| graph.get(url).and_then(|m| m.js())); - // fast check puts the same diagnostics in each entrypoint for the - // package (since it's all or nothing), so we only need to check - // the first one JS entrypoint - let Some(module) = js_exports.next() else { - // could happen if all the exports are JSON - return vec![]; - }; - - if let Some(diagnostics) = module.fast_check_diagnostics() { - let mut diagnostics = diagnostics.clone(); - diagnostics.sort_by_cached_key(|d| { - ( - d.specifier().clone(), - d.range().map(|r| r.range), - d.code().to_string(), - ) - }); - diagnostics - } else { - Vec::new() - } -} diff --git a/cli/tools/lint/rules/mod.rs b/cli/tools/lint/rules/mod.rs new file mode 100644 index 00000000000000..2669ffda15fd01 --- /dev/null +++ b/cli/tools/lint/rules/mod.rs @@ -0,0 +1,296 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use std::borrow::Cow; +use std::collections::HashSet; +use std::sync::Arc; + +use deno_ast::ModuleSpecifier; +use deno_config::deno_json::ConfigFile; +use deno_config::deno_json::LintRulesConfig; +use deno_config::workspace::WorkspaceResolver; +use deno_core::anyhow::bail; +use deno_core::error::AnyError; +use deno_graph::ModuleGraph; +use deno_lint::diagnostic::LintDiagnostic; +use deno_lint::rules::LintRule; + +use crate::resolver::SloppyImportsResolver; + +mod no_sloppy_imports; +mod no_slow_types; + +// used for publishing +pub use no_slow_types::collect_no_slow_type_diagnostics; + +pub trait PackageLintRule: std::fmt::Debug + Send + Sync { + fn code(&self) -> &'static str; + + fn tags(&self) -> &'static [&'static str] { + &[] + } + + fn docs(&self) -> &'static str; + + fn help_docs_url(&self) -> Cow<'static, str>; + + fn lint_package( + &self, + graph: &ModuleGraph, + entrypoints: &[ModuleSpecifier], + ) -> Vec; +} + +pub(super) trait ExtendedLintRule: LintRule { + /// If the rule supports the incremental cache. + fn supports_incremental_cache(&self) -> bool; + + fn help_docs_url(&self) -> Cow<'static, str>; + + fn into_base(self: Box) -> Box; +} + +pub enum FileOrPackageLintRule { + File(Box), + Package(Box), +} + +#[derive(Debug)] +enum CliLintRuleKind { + DenoLint(Box), + Extended(Box), + Package(Box), +} + +#[derive(Debug)] +pub struct CliLintRule(CliLintRuleKind); + +impl CliLintRule { + pub fn code(&self) -> &'static str { + use CliLintRuleKind::*; + match &self.0 { + DenoLint(rule) => rule.code(), + Extended(rule) => rule.code(), + Package(rule) => rule.code(), + } + } + + pub fn tags(&self) -> &'static [&'static str] { + use CliLintRuleKind::*; + match &self.0 { + DenoLint(rule) => rule.tags(), + Extended(rule) => rule.tags(), + Package(rule) => rule.tags(), + } + } + + pub fn docs(&self) -> &'static str { + use CliLintRuleKind::*; + match &self.0 { + DenoLint(rule) => rule.docs(), + Extended(rule) => rule.docs(), + Package(rule) => rule.docs(), + } + } + + pub fn help_docs_url(&self) -> Cow<'static, str> { + use CliLintRuleKind::*; + match &self.0 { + DenoLint(rule) => { + Cow::Owned(format!("https://lint.deno.land/rules/{}", rule.code())) + } + Extended(rule) => rule.help_docs_url(), + Package(rule) => rule.help_docs_url(), + } + } + + pub fn supports_incremental_cache(&self) -> bool { + use CliLintRuleKind::*; + match &self.0 { + DenoLint(_) => true, + Extended(rule) => rule.supports_incremental_cache(), + // graph rules don't go through the incremental cache, so allow it + Package(_) => true, + } + } + + pub fn into_file_or_pkg_rule(self) -> FileOrPackageLintRule { + use CliLintRuleKind::*; + match self.0 { + DenoLint(rule) => FileOrPackageLintRule::File(rule), + Extended(rule) => FileOrPackageLintRule::File(rule.into_base()), + Package(rule) => FileOrPackageLintRule::Package(rule), + } + } +} + +#[derive(Debug)] +pub struct ConfiguredRules { + pub all_rule_codes: HashSet<&'static str>, + pub rules: Vec, +} + +impl ConfiguredRules { + pub fn incremental_cache_state(&self) -> Option { + if self.rules.iter().any(|r| !r.supports_incremental_cache()) { + return None; + } + + // use a hash of the rule names in order to bust the cache + let mut codes = self.rules.iter().map(|r| r.code()).collect::>(); + // ensure this is stable by sorting it + codes.sort_unstable(); + Some(codes) + } +} + +pub struct LintRuleProvider { + sloppy_imports_resolver: Option>, + workspace_resolver: Option>, +} + +impl LintRuleProvider { + pub fn new( + sloppy_imports_resolver: Option>, + workspace_resolver: Option>, + ) -> Self { + Self { + sloppy_imports_resolver, + workspace_resolver, + } + } + + pub fn resolve_lint_rules_err_empty( + &self, + rules: LintRulesConfig, + maybe_config_file: Option<&ConfigFile>, + ) -> Result { + let lint_rules = self.resolve_lint_rules(rules, maybe_config_file); + if lint_rules.rules.is_empty() { + bail!("No rules have been configured") + } + Ok(lint_rules) + } + + pub fn resolve_lint_rules( + &self, + rules: LintRulesConfig, + maybe_config_file: Option<&ConfigFile>, + ) -> ConfiguredRules { + let deno_lint_rules = deno_lint::rules::get_all_rules(); + let cli_lint_rules = vec![CliLintRule(CliLintRuleKind::Extended( + Box::new(no_sloppy_imports::NoSloppyImportsRule::new( + self.sloppy_imports_resolver.clone(), + self.workspace_resolver.clone(), + )), + ))]; + let cli_graph_rules = vec![CliLintRule(CliLintRuleKind::Package( + Box::new(no_slow_types::NoSlowTypesRule), + ))]; + let mut all_rule_names = HashSet::with_capacity( + deno_lint_rules.len() + cli_lint_rules.len() + cli_graph_rules.len(), + ); + let all_rules = deno_lint_rules + .into_iter() + .map(|rule| CliLintRule(CliLintRuleKind::DenoLint(rule))) + .chain(cli_lint_rules) + .chain(cli_graph_rules) + .inspect(|rule| { + all_rule_names.insert(rule.code()); + }); + let rules = filtered_rules( + all_rules, + rules + .tags + .or_else(|| Some(get_default_tags(maybe_config_file))), + rules.exclude, + rules.include, + ); + ConfiguredRules { + rules, + all_rule_codes: all_rule_names, + } + } +} + +fn get_default_tags(maybe_config_file: Option<&ConfigFile>) -> Vec { + let mut tags = Vec::with_capacity(2); + tags.push("recommended".to_string()); + if maybe_config_file.map(|c| c.is_package()).unwrap_or(false) { + tags.push("jsr".to_string()); + } + tags +} + +fn filtered_rules( + all_rules: impl Iterator, + maybe_tags: Option>, + maybe_exclude: Option>, + maybe_include: Option>, +) -> Vec { + let tags_set = + maybe_tags.map(|tags| tags.into_iter().collect::>()); + + let mut rules = all_rules + .filter(|rule| { + let mut passes = if let Some(tags_set) = &tags_set { + rule + .tags() + .iter() + .any(|t| tags_set.contains(&t.to_string())) + } else { + true + }; + + if let Some(includes) = &maybe_include { + if includes.contains(&rule.code().to_owned()) { + passes |= true; + } + } + + if let Some(excludes) = &maybe_exclude { + if excludes.contains(&rule.code().to_owned()) { + passes &= false; + } + } + + passes + }) + .collect::>(); + + rules.sort_by_key(|r| r.code()); + + rules +} + +#[cfg(test)] +mod test { + use super::*; + use crate::args::LintRulesConfig; + + #[test] + fn recommended_rules_when_no_tags_in_config() { + let rules_config = LintRulesConfig { + exclude: Some(vec!["no-debugger".to_string()]), + include: None, + tags: None, + }; + let rules_provider = LintRuleProvider::new(None, None); + let rules = rules_provider.resolve_lint_rules(rules_config, None); + let mut rule_names = rules + .rules + .into_iter() + .map(|r| r.code().to_string()) + .collect::>(); + rule_names.sort(); + let mut recommended_rule_names = rules_provider + .resolve_lint_rules(Default::default(), None) + .rules + .into_iter() + .filter(|r| r.tags().iter().any(|t| *t == "recommended")) + .map(|r| r.code().to_string()) + .filter(|n| n != "no-debugger") + .collect::>(); + recommended_rule_names.sort(); + assert_eq!(rule_names, recommended_rule_names); + } +} diff --git a/cli/tools/lint/rules/no_sloppy_imports.md b/cli/tools/lint/rules/no_sloppy_imports.md new file mode 100644 index 00000000000000..08547c9da3dad4 --- /dev/null +++ b/cli/tools/lint/rules/no_sloppy_imports.md @@ -0,0 +1,20 @@ +Enforces specifying explicit references to paths in module specifiers. + +Non-explicit specifiers are ambiguous and require probing for the correct file +path on every run, which has a performance overhead. + +Note: This lint rule is only active when using `--unstable-sloppy-imports`. + +### Invalid: + +```typescript +import { add } from "./math/add"; +import { ConsoleLogger } from "./loggers"; +``` + +### Valid: + +```typescript +import { add } from "./math/add.ts"; +import { ConsoleLogger } from "./loggers/index.ts"; +``` diff --git a/cli/tools/lint/rules/no_sloppy_imports.rs b/cli/tools/lint/rules/no_sloppy_imports.rs new file mode 100644 index 00000000000000..b5e057bfc495d1 --- /dev/null +++ b/cli/tools/lint/rules/no_sloppy_imports.rs @@ -0,0 +1,214 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use std::borrow::Cow; +use std::cell::RefCell; +use std::collections::HashMap; +use std::sync::Arc; + +use deno_ast::SourceRange; +use deno_config::workspace::WorkspaceResolver; +use deno_core::anyhow::anyhow; +use deno_graph::source::ResolutionMode; +use deno_graph::source::ResolveError; +use deno_graph::Range; +use deno_lint::diagnostic::LintDiagnosticDetails; +use deno_lint::diagnostic::LintDiagnosticRange; +use deno_lint::diagnostic::LintFix; +use deno_lint::diagnostic::LintFixChange; +use deno_lint::rules::LintRule; +use text_lines::LineAndColumnIndex; + +use crate::graph_util::CliJsrUrlProvider; +use crate::resolver::SloppyImportsResolution; +use crate::resolver::SloppyImportsResolver; + +use super::ExtendedLintRule; + +#[derive(Debug)] +pub struct NoSloppyImportsRule { + sloppy_imports_resolver: Option>, + // None for making printing out the lint rules easy + workspace_resolver: Option>, +} + +impl NoSloppyImportsRule { + pub fn new( + sloppy_imports_resolver: Option>, + workspace_resolver: Option>, + ) -> Self { + NoSloppyImportsRule { + sloppy_imports_resolver, + workspace_resolver, + } + } +} + +const CODE: &str = "no-sloppy-imports"; +const DOCS_URL: &str = "https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports"; + +impl ExtendedLintRule for NoSloppyImportsRule { + fn supports_incremental_cache(&self) -> bool { + // only allow the incremental cache when we don't + // do sloppy import resolution because sloppy import + // resolution requires knowing about the surrounding files + // in addition to the current one + self.sloppy_imports_resolver.is_none() || self.workspace_resolver.is_none() + } + + fn help_docs_url(&self) -> Cow<'static, str> { + Cow::Borrowed(DOCS_URL) + } + + fn into_base(self: Box) -> Box { + self + } +} + +impl LintRule for NoSloppyImportsRule { + fn lint_program_with_ast_view<'view>( + &self, + context: &mut deno_lint::context::Context<'view>, + _program: deno_lint::Program<'view>, + ) { + let Some(workspace_resolver) = &self.workspace_resolver else { + return; + }; + let Some(sloppy_imports_resolver) = &self.sloppy_imports_resolver else { + return; + }; + if context.specifier().scheme() != "file" { + return; + } + + let resolver = SloppyImportCaptureResolver { + workspace_resolver, + sloppy_imports_resolver, + captures: Default::default(), + }; + + deno_graph::parse_module_from_ast(deno_graph::ParseModuleFromAstOptions { + graph_kind: deno_graph::GraphKind::All, + specifier: context.specifier().clone(), + maybe_headers: None, + parsed_source: context.parsed_source(), + // ignore resolving dynamic imports like import(`./dir/${something}`) + file_system: &deno_graph::source::NullFileSystem, + jsr_url_provider: &CliJsrUrlProvider, + maybe_resolver: Some(&resolver), + // don't bother resolving npm specifiers + maybe_npm_resolver: None, + }); + + for (range, sloppy_import) in resolver.captures.borrow_mut().drain() { + let start_range = + context.text_info().loc_to_source_pos(LineAndColumnIndex { + line_index: range.start.line, + column_index: range.start.character, + }); + let end_range = + context.text_info().loc_to_source_pos(LineAndColumnIndex { + line_index: range.end.line, + column_index: range.end.character, + }); + let source_range = SourceRange::new(start_range, end_range); + context.add_diagnostic_details( + Some(LintDiagnosticRange { + range: source_range, + description: None, + text_info: context.text_info().clone(), + }), + LintDiagnosticDetails { + message: "Sloppy imports are not allowed.".to_string(), + code: CODE.to_string(), + custom_docs_url: Some(DOCS_URL.to_string()), + fixes: context + .specifier() + .make_relative(sloppy_import.as_specifier()) + .map(|relative| { + vec![LintFix { + description: Cow::Owned(sloppy_import.as_quick_fix_message()), + changes: vec![LintFixChange { + new_text: Cow::Owned({ + let relative = if relative.starts_with("../") { + relative + } else { + format!("./{}", relative) + }; + let current_text = + context.text_info().range_text(&source_range); + if current_text.starts_with('"') { + format!("\"{}\"", relative) + } else if current_text.starts_with('\'') { + format!("'{}'", relative) + } else { + relative + } + }), + range: source_range, + }], + }] + }) + .unwrap_or_default(), + hint: None, + info: vec![], + }, + ); + } + } + + fn code(&self) -> &'static str { + CODE + } + + fn docs(&self) -> &'static str { + include_str!("no_sloppy_imports.md") + } + + fn tags(&self) -> &'static [&'static str] { + &["recommended"] + } +} + +#[derive(Debug)] +struct SloppyImportCaptureResolver<'a> { + workspace_resolver: &'a WorkspaceResolver, + sloppy_imports_resolver: &'a SloppyImportsResolver, + captures: RefCell>, +} + +impl<'a> deno_graph::source::Resolver for SloppyImportCaptureResolver<'a> { + fn resolve( + &self, + specifier_text: &str, + referrer_range: &Range, + mode: ResolutionMode, + ) -> Result { + let resolution = self + .workspace_resolver + .resolve(specifier_text, &referrer_range.specifier) + .map_err(|err| ResolveError::Other(err.into()))?; + + match resolution { + deno_config::workspace::MappedResolution::Normal(specifier) + | deno_config::workspace::MappedResolution::ImportMap(specifier) => { + match self.sloppy_imports_resolver.resolve(&specifier, mode) { + Some(res) => { + self + .captures + .borrow_mut() + .entry(referrer_range.clone()) + .or_insert_with(|| res.clone()); + Ok(res.into_specifier()) + } + None => Ok(specifier), + } + } + deno_config::workspace::MappedResolution::WorkspaceNpmPackage { + .. + } + | deno_config::workspace::MappedResolution::PackageJson { .. } => { + Err(ResolveError::Other(anyhow!(""))) + } + } + } +} diff --git a/cli/tools/lint/rules/no_slow_types.md b/cli/tools/lint/rules/no_slow_types.md new file mode 100644 index 00000000000000..d0764d8659c433 --- /dev/null +++ b/cli/tools/lint/rules/no_slow_types.md @@ -0,0 +1,3 @@ +Enforces using types that are explicit or can be simply inferred. + +Read more: https://jsr.io/docs/about-slow-types diff --git a/cli/tools/lint/rules/no_slow_types.rs b/cli/tools/lint/rules/no_slow_types.rs new file mode 100644 index 00000000000000..bc3f835b179be7 --- /dev/null +++ b/cli/tools/lint/rules/no_slow_types.rs @@ -0,0 +1,98 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use std::borrow::Cow; + +use deno_ast::diagnostics::Diagnostic; +use deno_ast::ModuleSpecifier; +use deno_graph::FastCheckDiagnostic; +use deno_graph::ModuleGraph; +use deno_lint::diagnostic::LintDiagnostic; +use deno_lint::diagnostic::LintDiagnosticDetails; +use deno_lint::diagnostic::LintDiagnosticRange; + +use super::PackageLintRule; + +const CODE: &str = "no-slow-types"; + +#[derive(Debug)] +pub struct NoSlowTypesRule; + +impl PackageLintRule for NoSlowTypesRule { + fn code(&self) -> &'static str { + CODE + } + + fn tags(&self) -> &'static [&'static str] { + &["jsr"] + } + + fn docs(&self) -> &'static str { + include_str!("no_slow_types.md") + } + + fn help_docs_url(&self) -> Cow<'static, str> { + Cow::Borrowed("https://jsr.io/docs/about-slow-types") + } + + fn lint_package( + &self, + graph: &ModuleGraph, + entrypoints: &[ModuleSpecifier], + ) -> Vec { + collect_no_slow_type_diagnostics(graph, entrypoints) + .into_iter() + .map(|d| LintDiagnostic { + specifier: d.specifier().clone(), + range: d.range().map(|range| LintDiagnosticRange { + text_info: range.text_info.clone(), + range: range.range, + description: d.range_description().map(|r| r.to_string()), + }), + details: LintDiagnosticDetails { + message: d.message().to_string(), + code: CODE.to_string(), + hint: d.hint().map(|h| h.to_string()), + info: d + .info() + .iter() + .map(|info| Cow::Owned(info.to_string())) + .collect(), + fixes: vec![], + custom_docs_url: d.docs_url().map(|u| u.into_owned()), + }, + }) + .collect() + } +} + +/// Collects diagnostics from the module graph for the +/// given package's export URLs. +pub fn collect_no_slow_type_diagnostics( + graph: &ModuleGraph, + package_export_urls: &[ModuleSpecifier], +) -> Vec { + let mut js_exports = package_export_urls + .iter() + .filter_map(|url| graph.get(url).and_then(|m| m.js())); + // fast check puts the same diagnostics in each entrypoint for the + // package (since it's all or nothing), so we only need to check + // the first one JS entrypoint + let Some(module) = js_exports.next() else { + // could happen if all the exports are JSON + return vec![]; + }; + + if let Some(diagnostics) = module.fast_check_diagnostics() { + let mut diagnostics = diagnostics.clone(); + diagnostics.sort_by_cached_key(|d| { + ( + d.specifier().clone(), + d.range().map(|r| r.range), + d.code().to_string(), + ) + }); + diagnostics + } else { + Vec::new() + } +} diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index c9b742588b89a9..586327821a2949 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -45,7 +45,7 @@ use crate::graph_util::ModuleGraphCreator; use crate::http_util::HttpClient; use crate::resolver::SloppyImportsResolver; use crate::tools::check::CheckOptions; -use crate::tools::lint::no_slow_types; +use crate::tools::lint::collect_no_slow_type_diagnostics; use crate::tools::registry::diagnostics::PublishDiagnostic; use crate::tools::registry::diagnostics::PublishDiagnosticsCollector; use crate::util::display::human_size; @@ -341,7 +341,7 @@ impl PublishPreparer { for package in package_configs { let export_urls = package.config_file.resolve_export_value_urls()?; let diagnostics = - no_slow_types::collect_no_slow_type_diagnostics(&export_urls, &graph); + collect_no_slow_type_diagnostics(&graph, &export_urls); if !diagnostics.is_empty() { any_pkg_had_diagnostics = true; for diagnostic in diagnostics { diff --git a/cli/tools/registry/unfurl.rs b/cli/tools/registry/unfurl.rs index f7c1049ca1051d..a28ba445a32156 100644 --- a/cli/tools/registry/unfurl.rs +++ b/cli/tools/registry/unfurl.rs @@ -177,8 +177,8 @@ impl SpecifierUnfurler { if let Some(sloppy_imports_resolver) = &self.sloppy_imports_resolver { sloppy_imports_resolver .resolve(&resolved, deno_graph::source::ResolutionMode::Execution) - .as_specifier() - .clone() + .map(|res| res.into_specifier()) + .unwrap_or(resolved) } else { resolved }; diff --git a/cli/tools/run/mod.rs b/cli/tools/run/mod.rs index baa8e72db0f1cf..65044fbad3abc3 100644 --- a/cli/tools/run/mod.rs +++ b/cli/tools/run/mod.rs @@ -45,13 +45,6 @@ To grant permissions, set them before the script argument. For example: let deno_dir = factory.deno_dir()?; let http_client = factory.http_client_provider(); - if cli_options.unstable_sloppy_imports() { - log::warn!( - "{} Sloppy imports are not recommended and have a negative impact on performance.", - crate::colors::yellow("Warning"), - ); - } - // Run a background task that checks for available upgrades or output // if an earlier run of this background task found a new version of Deno. #[cfg(feature = "upgrade")] diff --git a/tests/integration/check_tests.rs b/tests/integration/check_tests.rs index 78ab859f925e75..b560b352eef25a 100644 --- a/tests/integration/check_tests.rs +++ b/tests/integration/check_tests.rs @@ -392,105 +392,3 @@ fn npm_module_check_then_error() { .assert_matches_text("Check [WILDCARD]main.ts\nerror: TS2305[WILDCARD]has no exported member 'oldName'[WILDCARD]") .assert_exit_code(1); } - -#[test] -fn test_unstable_sloppy_imports_dts_files() { - let context = TestContextBuilder::new().use_temp_cwd().build(); - let temp_dir = context.temp_dir(); - temp_dir.write("a.ts", "export class A {}"); // resolves this - temp_dir.write("a.d.ts", "export class A2 {}"); - - temp_dir.write("b.js", "export class B {}"); - temp_dir.write("b.d.ts", "export class B2 {}"); // this - - temp_dir.write("c.mts", "export class C {}"); // this - temp_dir.write("c.d.mts", "export class C2 {}"); - - temp_dir.write("d.mjs", "export class D {}"); - temp_dir.write("d.d.mts", "export class D2 {}"); // this - - let temp_dir = temp_dir.path(); - - let dir = temp_dir.join("dir_ts"); - dir.create_dir_all(); - dir.join("index.ts").write("export class Dir {}"); // this - dir.join("index.d.ts").write("export class Dir2 {}"); - - let dir = temp_dir.join("dir_js"); - dir.create_dir_all(); - dir.join("index.js").write("export class Dir {}"); - dir.join("index.d.ts").write("export class Dir2 {}"); // this - - let dir = temp_dir.join("dir_mts"); - dir.create_dir_all(); - dir.join("index.mts").write("export class Dir {}"); // this - dir.join("index.d.ts").write("export class Dir2 {}"); - - let dir = temp_dir.join("dir_mjs"); - dir.create_dir_all(); - dir.join("index.mjs").write("export class Dir {}"); - dir.join("index.d.ts").write("export class Dir2 {}"); // this - - temp_dir.join("main.ts").write( - r#"import * as a from "./a.js"; -import * as b from "./b.js"; -import * as c from "./c.mjs"; -import * as d from "./d.mjs"; - -console.log(a.A); -console.log(b.B2); -console.log(c.C); -console.log(d.D2); - -import * as a2 from "./a"; -import * as b2 from "./b"; -import * as c2 from "./c"; -import * as d2 from "./d"; - -console.log(a2.A); -console.log(b2.B2); -console.log(c2.C); -console.log(d2.D2); - -import * as dirTs from "./dir_ts"; -import * as dirJs from "./dir_js"; -import * as dirMts from "./dir_mts"; -import * as dirMjs from "./dir_mjs"; - -console.log(dirTs.Dir); -console.log(dirJs.Dir2); -console.log(dirMts.Dir); -console.log(dirMjs.Dir2); -"#, - ); - - context - .new_command() - .args("check --unstable-sloppy-imports main.ts") - .run() - .assert_matches_text( - r#"Warning Sloppy module resolution (hint: update .js extension to .ts) - at file:///[WILDCARD]/main.ts:1:20 -Warning Sloppy module resolution (hint: update .mjs extension to .mts) - at file:///[WILDCARD]/main.ts:3:20 -Warning Sloppy module resolution (hint: add .ts extension) - at file:///[WILDCARD]/main.ts:11:21 -Warning Sloppy module resolution (hint: add .js extension) - at file:///[WILDCARD]/main.ts:12:21 -Warning Sloppy module resolution (hint: add .mts extension) - at file:///[WILDCARD]/main.ts:13:21 -Warning Sloppy module resolution (hint: add .mjs extension) - at file:///[WILDCARD]/main.ts:14:21 -Warning Sloppy module resolution (hint: specify path to index.ts file in directory instead) - at file:///[WILDCARD]/main.ts:21:24 -Warning Sloppy module resolution (hint: specify path to index.js file in directory instead) - at file:///[WILDCARD]/main.ts:22:24 -Warning Sloppy module resolution (hint: specify path to index.mts file in directory instead) - at file:///[WILDCARD]/main.ts:23:25 -Warning Sloppy module resolution (hint: specify path to index.mjs file in directory instead) - at file:///[WILDCARD]/main.ts:24:25 -Check [WILDCARD]main.ts -"#, - ) - .assert_exit_code(0); -} diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index e8904942de79ce..cbc175ec6a9c46 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -14459,7 +14459,67 @@ fn lsp_sloppy_imports() { }, })); - assert_eq!(json!(diagnostics.all()), json!([])); + assert_eq!( + json!(diagnostics.all()), + json!([{ + "range": { + "start": { "line": 0, "character": 19 }, + "end": { "line": 0, "character": 24 } + }, + "severity": 2, + "code": "no-sloppy-imports", + "source": "deno-lint", + "message": "Sloppy imports are not allowed.", + "data": [{ + "description": "Add a '.ts' extension.", + "changes": [{ + "range": { + "start": { "line": 0, "character": 19 }, + "end": { "line": 0, "character": 24 }, + }, + "new_text": "'./a.ts'" + }] + }] + }, { + "range": { + "start": { "line": 1, "character": 19 }, + "end": { "line": 1, "character": 27 } + }, + "severity": 2, + "code": "no-sloppy-imports", + "source": "deno-lint", + "message": "Sloppy imports are not allowed.", + "data": [{ + "description": "Change the extension to '.ts'.", + "changes": [{ + "range": { + "start": { "line": 1, "character": 19 }, + "end": { "line": 1, "character": 27 }, + }, + "new_text": "'./b.ts'" + }] + }] + }, { + "range": { + "start": { "line": 2, "character": 19 }, + "end": { "line": 2, "character": 27 } + }, + "severity": 2, + "code": "no-sloppy-imports", + "source": "deno-lint", + "message": "Sloppy imports are not allowed.", + "data": [{ + "description": "Change the extension to '.d.ts'.", + "changes": [{ + "range": { + "start": { "line": 2, "character": 19 }, + "end": { "line": 2, "character": 27 }, + }, + "new_text": "'./c.d.ts'" + }] + }] + }]) + ); client.shutdown(); } @@ -14488,11 +14548,33 @@ fn lsp_sloppy_imports_prefers_dts() { "import { foo } from './a.js';\nconsole.log(foo);", ); let diagnostics = client.did_open_file(&file); - // no warnings because "a.js" exists - assert_eq!(diagnostics.all().len(), 0); + // no other warnings because "a.js" exists + assert_eq!( + json!(diagnostics.all()), + json!([{ + "range": { + "start": { "line": 0, "character": 20 }, + "end": { "line": 0, "character": 28 } + }, + "severity": 2, + "code": "no-sloppy-imports", + "source": "deno-lint", + "message": "Sloppy imports are not allowed.", + "data": [{ + "description": "Change the extension to '.d.ts'.", + "changes": [{ + "range": { + "start": { "line": 0, "character": 20 }, + "end": { "line": 0, "character": 28 }, + }, + "new_text": "'./a.d.ts'" + }] + }] + }]) + ); let diagnostics = client.did_open_file(&a_dts); - assert_eq!(diagnostics.all().len(), 0, "Got {:#?}", diagnostics.all()); + assert_eq!(json!(diagnostics.for_file(&a_dts.uri())), json!([])); let response = client.write_request( "textDocument/references", diff --git a/tests/integration/run_tests.rs b/tests/integration/run_tests.rs index 73d98aff54fb7f..7a6ee61cf15e89 100644 --- a/tests/integration/run_tests.rs +++ b/tests/integration/run_tests.rs @@ -4851,86 +4851,6 @@ itest!(unsafe_proto_flag { exit_code: 0, }); -#[test] -fn test_unstable_sloppy_imports() { - let context = TestContextBuilder::new().use_temp_cwd().build(); - let temp_dir = context.temp_dir(); - temp_dir.write("a.ts", "export class A {}"); - temp_dir.write("b.js", "export class B {}"); - temp_dir.write("c.mts", "export class C {}"); - temp_dir.write("d.mjs", "export class D {}"); - temp_dir.write("e.tsx", "export class E {}"); - temp_dir.write("f.jsx", "export class F {}"); - let dir = temp_dir.path().join("dir"); - dir.create_dir_all(); - dir.join("index.tsx").write("export class G {}"); - temp_dir.write( - "main.ts", - r#"import * as a from "./a.js"; -import * as b from "./b"; -import * as c from "./c"; -import * as d from "./d"; -import * as e from "./e"; -import * as e2 from "./e.js"; -import * as f from "./f"; -import * as g from "./dir"; -console.log(a.A); -console.log(b.B); -console.log(c.C); -console.log(d.D); -console.log(e.E); -console.log(e2.E); -console.log(f.F); -console.log(g.G); -"#, - ); - - // run without sloppy imports - context - .new_command() - .args("run main.ts") - .run() - .assert_matches_text(r#"error: Module not found "file:///[WILDCARD]/a.js". Maybe change the extension to '.ts' or run with --unstable-sloppy-imports - at file:///[WILDCARD]/main.ts:1:20 -"#) - .assert_exit_code(1); - - // now run with sloppy imports - temp_dir.write("deno.json", r#"{ "unstable": ["sloppy-imports"] }"#); - context - .new_command() - .args("run main.ts") - .run() - .assert_matches_text( - "Warning Sloppy imports are not recommended and have a negative impact on performance. -Warning Sloppy module resolution (hint: update .js extension to .ts) - at file:///[WILDCARD]/main.ts:1:20 -Warning Sloppy module resolution (hint: add .js extension) - at file:///[WILDCARD]/main.ts:2:20 -Warning Sloppy module resolution (hint: add .mts extension) - at file:///[WILDCARD]/main.ts:3:20 -Warning Sloppy module resolution (hint: add .mjs extension) - at file:///[WILDCARD]/main.ts:4:20 -Warning Sloppy module resolution (hint: add .tsx extension) - at file:///[WILDCARD]/main.ts:5:20 -Warning Sloppy module resolution (hint: update .js extension to .tsx) - at file:///[WILDCARD]/main.ts:6:21 -Warning Sloppy module resolution (hint: add .jsx extension) - at file:///[WILDCARD]/main.ts:7:20 -Warning Sloppy module resolution (hint: specify path to index.tsx file in directory instead) - at file:///[WILDCARD]/main.ts:8:20 -[class A] -[class B] -[class C] -[class D] -[class E] -[class E] -[class F] -[class G] -", - ); -} - itest!(unstable_temporal_api { args: "run --no-config --unstable-temporal --check run/unstable_temporal_api/main.ts", output: "run/unstable_temporal_api/main.out", diff --git a/tests/specs/lint/sloppy_imports_dts/__test__.jsonc b/tests/specs/lint/sloppy_imports_dts/__test__.jsonc new file mode 100644 index 00000000000000..2650d7ec202e13 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/__test__.jsonc @@ -0,0 +1,34 @@ +{ + "tests": { + "check": { + "args": "check --unstable-sloppy-imports main.ts", + "output": "check.out" + }, + "run": { + "args": "run --unstable-sloppy-imports main.ts", + "output": "run.out" + }, + "lint": { + "args": "lint --unstable-sloppy-imports", + "output": "lint.out", + "exitCode": 1 + }, + // try fixing the lint issues and then ensure deno check and run still work + "lint_fix": { + "tempDir": true, + "steps": [{ + "args": "lint --unstable-sloppy-imports --fix", + "output": "Checked 17 files\n" + }, { + "args": "lint --unstable-sloppy-imports", + "output": "Checked 17 files\n" + }, { + "args": "check --unstable-sloppy-imports main.ts", + "output": "check.out" + }, { + "args": "run --unstable-sloppy-imports main.ts", + "output": "run.out" + }] + } + } +} diff --git a/tests/specs/lint/sloppy_imports_dts/a.d.ts b/tests/specs/lint/sloppy_imports_dts/a.d.ts new file mode 100644 index 00000000000000..b0c84214f3124f --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/a.d.ts @@ -0,0 +1 @@ +export class A2 {} diff --git a/tests/specs/lint/sloppy_imports_dts/a.ts b/tests/specs/lint/sloppy_imports_dts/a.ts new file mode 100644 index 00000000000000..1e14df5446ad85 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/a.ts @@ -0,0 +1 @@ +export class A {} diff --git a/tests/specs/lint/sloppy_imports_dts/b.d.ts b/tests/specs/lint/sloppy_imports_dts/b.d.ts new file mode 100644 index 00000000000000..522d6784df92a5 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/b.d.ts @@ -0,0 +1 @@ +export class B2 {} diff --git a/tests/specs/lint/sloppy_imports_dts/b.js b/tests/specs/lint/sloppy_imports_dts/b.js new file mode 100644 index 00000000000000..1aa41a54a30944 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/b.js @@ -0,0 +1 @@ +export class B {} diff --git a/tests/specs/lint/sloppy_imports_dts/c.d.mts b/tests/specs/lint/sloppy_imports_dts/c.d.mts new file mode 100644 index 00000000000000..efd6dcc73555f6 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/c.d.mts @@ -0,0 +1 @@ +export class C2 {} diff --git a/tests/specs/lint/sloppy_imports_dts/c.mts b/tests/specs/lint/sloppy_imports_dts/c.mts new file mode 100644 index 00000000000000..1ec0ebf40c59b1 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/c.mts @@ -0,0 +1 @@ +export class C {} diff --git a/tests/specs/lint/sloppy_imports_dts/check.out b/tests/specs/lint/sloppy_imports_dts/check.out new file mode 100644 index 00000000000000..1830c3186d0669 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/check.out @@ -0,0 +1 @@ +Check file:///[WILDLINE]/main.ts diff --git a/tests/specs/lint/sloppy_imports_dts/d.d.mts b/tests/specs/lint/sloppy_imports_dts/d.d.mts new file mode 100644 index 00000000000000..629b29e308f8d4 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/d.d.mts @@ -0,0 +1 @@ +export class D2 {} diff --git a/tests/specs/lint/sloppy_imports_dts/d.mjs b/tests/specs/lint/sloppy_imports_dts/d.mjs new file mode 100644 index 00000000000000..01b958f66df360 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/d.mjs @@ -0,0 +1 @@ +export class D {} diff --git a/tests/specs/lint/sloppy_imports_dts/dir_js/index.d.ts b/tests/specs/lint/sloppy_imports_dts/dir_js/index.d.ts new file mode 100644 index 00000000000000..23cac850061867 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/dir_js/index.d.ts @@ -0,0 +1 @@ +export class Dir2 {} diff --git a/tests/specs/lint/sloppy_imports_dts/dir_js/index.js b/tests/specs/lint/sloppy_imports_dts/dir_js/index.js new file mode 100644 index 00000000000000..f31e303be7c87c --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/dir_js/index.js @@ -0,0 +1 @@ +export class Dir {} diff --git a/tests/specs/lint/sloppy_imports_dts/dir_mjs/index.d.ts b/tests/specs/lint/sloppy_imports_dts/dir_mjs/index.d.ts new file mode 100644 index 00000000000000..23cac850061867 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/dir_mjs/index.d.ts @@ -0,0 +1 @@ +export class Dir2 {} diff --git a/tests/specs/lint/sloppy_imports_dts/dir_mjs/index.mjs b/tests/specs/lint/sloppy_imports_dts/dir_mjs/index.mjs new file mode 100644 index 00000000000000..f31e303be7c87c --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/dir_mjs/index.mjs @@ -0,0 +1 @@ +export class Dir {} diff --git a/tests/specs/lint/sloppy_imports_dts/dir_mts/index.d.ts b/tests/specs/lint/sloppy_imports_dts/dir_mts/index.d.ts new file mode 100644 index 00000000000000..23cac850061867 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/dir_mts/index.d.ts @@ -0,0 +1 @@ +export class Dir2 {} diff --git a/tests/specs/lint/sloppy_imports_dts/dir_mts/index.mts b/tests/specs/lint/sloppy_imports_dts/dir_mts/index.mts new file mode 100644 index 00000000000000..f31e303be7c87c --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/dir_mts/index.mts @@ -0,0 +1 @@ +export class Dir {} diff --git a/tests/specs/lint/sloppy_imports_dts/dir_ts/index.d.ts b/tests/specs/lint/sloppy_imports_dts/dir_ts/index.d.ts new file mode 100644 index 00000000000000..23cac850061867 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/dir_ts/index.d.ts @@ -0,0 +1 @@ +export class Dir2 {} diff --git a/tests/specs/lint/sloppy_imports_dts/dir_ts/index.ts b/tests/specs/lint/sloppy_imports_dts/dir_ts/index.ts new file mode 100644 index 00000000000000..f31e303be7c87c --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/dir_ts/index.ts @@ -0,0 +1 @@ +export class Dir {} diff --git a/tests/specs/lint/sloppy_imports_dts/lint.out b/tests/specs/lint/sloppy_imports_dts/lint.out new file mode 100644 index 00000000000000..d891676ac019bb --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/lint.out @@ -0,0 +1,110 @@ +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:1:20 + | +1 | import * as a from "./a.js"; + | ^^^^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:2:20 + | +2 | import * as b from "./b.js"; + | ^^^^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:3:20 + | +3 | import * as c from "./c.mjs"; + | ^^^^^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:4:20 + | +4 | import * as d from "./d.mjs"; + | ^^^^^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:11:21 + | +11 | import * as a2 from "./a"; + | ^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:12:21 + | +12 | import * as b2 from "./b"; + | ^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:13:21 + | +13 | import * as c2 from "./c"; + | ^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:14:21 + | +14 | import * as d2 from "./d"; + | ^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:21:24 + | +21 | import * as dirTs from "./dir_ts"; + | ^^^^^^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:22:24 + | +22 | import * as dirJs from "./dir_js"; + | ^^^^^^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:23:25 + | +23 | import * as dirMts from "./dir_mts"; + | ^^^^^^^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:24:25 + | +24 | import * as dirMjs from "./dir_mjs"; + | ^^^^^^^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +Found 12 problems (12 fixable via --fix) +Checked 17 files diff --git a/tests/specs/lint/sloppy_imports_dts/main.ts b/tests/specs/lint/sloppy_imports_dts/main.ts new file mode 100644 index 00000000000000..2261eae581b443 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/main.ts @@ -0,0 +1,29 @@ +import * as a from "./a.js"; +import * as b from "./b.js"; +import * as c from "./c.mjs"; +import * as d from "./d.mjs"; + +console.log(a.A); +console.log(b.B2); +console.log(c.C); +console.log(d.D2); + +import * as a2 from "./a"; +import * as b2 from "./b"; +import * as c2 from "./c"; +import * as d2 from "./d"; + +console.log(a2.A); +console.log(b2.B2); +console.log(c2.C); +console.log(d2.D2); + +import * as dirTs from "./dir_ts"; +import * as dirJs from "./dir_js"; +import * as dirMts from "./dir_mts"; +import * as dirMjs from "./dir_mjs"; + +console.log(dirTs.Dir); +console.log(dirJs.Dir2); +console.log(dirMts.Dir); +console.log(dirMjs.Dir2); diff --git a/tests/specs/lint/sloppy_imports_dts/run.out b/tests/specs/lint/sloppy_imports_dts/run.out new file mode 100644 index 00000000000000..665534c646d47f --- /dev/null +++ b/tests/specs/lint/sloppy_imports_dts/run.out @@ -0,0 +1,12 @@ +[class A] +undefined +[class C] +undefined +[class A] +undefined +[class C] +undefined +[class Dir] +undefined +[class Dir] +undefined diff --git a/tests/specs/lint/sloppy_imports_no_incremental_cache/__test__.jsonc b/tests/specs/lint/sloppy_imports_no_incremental_cache/__test__.jsonc new file mode 100644 index 00000000000000..e4596fd212a08e --- /dev/null +++ b/tests/specs/lint/sloppy_imports_no_incremental_cache/__test__.jsonc @@ -0,0 +1,20 @@ +{ + "tempDir": true, + "steps": [{ + "args": "lint main.ts", + "output": "Checked 1 file\n" + }, { + "args": "lint --unstable-sloppy-imports main.ts", + "output": "Checked 1 file\n" + }, { + "args": [ + "eval", + "Deno.renameSync('file.js', 'file.ts')" + ], + "output": "" + }, { + "args": "lint --unstable-sloppy-imports main.ts", + "output": "fail_js_to_ts.out", + "exitCode": 1 + }] +} diff --git a/tests/specs/lint/sloppy_imports_no_incremental_cache/fail_js_to_ts.out b/tests/specs/lint/sloppy_imports_no_incremental_cache/fail_js_to_ts.out new file mode 100644 index 00000000000000..7321fd184bacd7 --- /dev/null +++ b/tests/specs/lint/sloppy_imports_no_incremental_cache/fail_js_to_ts.out @@ -0,0 +1,11 @@ +error[no-sloppy-imports]: Sloppy imports are not allowed. + --> [WILDLINE]main.ts:1:23 + | +1 | import * as file from "./file.js"; + | ^^^^^^^^^^^ + + docs: https://docs.deno.com/runtime/manual/tools/unstable_flags/#--unstable-sloppy-imports + + +Found 1 problem (1 fixable via --fix) +Checked 1 file diff --git a/tests/specs/lint/sloppy_imports_no_incremental_cache/file.js b/tests/specs/lint/sloppy_imports_no_incremental_cache/file.js new file mode 100644 index 00000000000000..23f12c5f6ed82a --- /dev/null +++ b/tests/specs/lint/sloppy_imports_no_incremental_cache/file.js @@ -0,0 +1 @@ +export class File {} diff --git a/tests/specs/lint/sloppy_imports_no_incremental_cache/main.ts b/tests/specs/lint/sloppy_imports_no_incremental_cache/main.ts new file mode 100644 index 00000000000000..873f4f4832055d --- /dev/null +++ b/tests/specs/lint/sloppy_imports_no_incremental_cache/main.ts @@ -0,0 +1,3 @@ +import * as file from "./file.js"; + +console.log(file); diff --git a/tests/specs/publish/sloppy_imports/sloppy_imports.out b/tests/specs/publish/sloppy_imports/sloppy_imports.out index bfa258b9399989..9371e9bb6eddda 100644 --- a/tests/specs/publish/sloppy_imports/sloppy_imports.out +++ b/tests/specs/publish/sloppy_imports/sloppy_imports.out @@ -1,5 +1,3 @@ -Warning Sloppy module resolution (hint: specify path to index.ts file in directory instead) - at file:///[WILDCARD]/mod.ts:1:20 Check file:///[WILDCARD]/mod.ts Checking for slow types in the public API... Check file:///[WILDCARD]/mod.ts diff --git a/tests/specs/run/sloppy_imports/__test__.jsonc b/tests/specs/run/sloppy_imports/__test__.jsonc new file mode 100644 index 00000000000000..79aaaba69d8fa6 --- /dev/null +++ b/tests/specs/run/sloppy_imports/__test__.jsonc @@ -0,0 +1,10 @@ +{ + "steps": [{ + "args": "run main.ts", + "output": "no_sloppy.out", + "exitCode": 1 + }, { + "args": "run --unstable-sloppy-imports main.ts", + "output": "sloppy.out" + }] +} diff --git a/tests/specs/run/sloppy_imports/a.ts b/tests/specs/run/sloppy_imports/a.ts new file mode 100644 index 00000000000000..1e14df5446ad85 --- /dev/null +++ b/tests/specs/run/sloppy_imports/a.ts @@ -0,0 +1 @@ +export class A {} diff --git a/tests/specs/run/sloppy_imports/b.js b/tests/specs/run/sloppy_imports/b.js new file mode 100644 index 00000000000000..1aa41a54a30944 --- /dev/null +++ b/tests/specs/run/sloppy_imports/b.js @@ -0,0 +1 @@ +export class B {} diff --git a/tests/specs/run/sloppy_imports/c.mts b/tests/specs/run/sloppy_imports/c.mts new file mode 100644 index 00000000000000..1ec0ebf40c59b1 --- /dev/null +++ b/tests/specs/run/sloppy_imports/c.mts @@ -0,0 +1 @@ +export class C {} diff --git a/tests/specs/run/sloppy_imports/d.mjs b/tests/specs/run/sloppy_imports/d.mjs new file mode 100644 index 00000000000000..01b958f66df360 --- /dev/null +++ b/tests/specs/run/sloppy_imports/d.mjs @@ -0,0 +1 @@ +export class D {} diff --git a/tests/specs/run/sloppy_imports/dir/index.tsx b/tests/specs/run/sloppy_imports/dir/index.tsx new file mode 100644 index 00000000000000..d679ef9a9ed914 --- /dev/null +++ b/tests/specs/run/sloppy_imports/dir/index.tsx @@ -0,0 +1 @@ +export class G {} diff --git a/tests/specs/run/sloppy_imports/e.tsx b/tests/specs/run/sloppy_imports/e.tsx new file mode 100644 index 00000000000000..70e8d4378ea49f --- /dev/null +++ b/tests/specs/run/sloppy_imports/e.tsx @@ -0,0 +1 @@ +export class E {} diff --git a/tests/specs/run/sloppy_imports/f.jsx b/tests/specs/run/sloppy_imports/f.jsx new file mode 100644 index 00000000000000..cee3fd259015f2 --- /dev/null +++ b/tests/specs/run/sloppy_imports/f.jsx @@ -0,0 +1 @@ +export class F {} diff --git a/tests/specs/run/sloppy_imports/main.ts b/tests/specs/run/sloppy_imports/main.ts new file mode 100644 index 00000000000000..3bdc3fe01eac41 --- /dev/null +++ b/tests/specs/run/sloppy_imports/main.ts @@ -0,0 +1,16 @@ +import * as a from "./a.js"; +import * as b from "./b"; +import * as c from "./c"; +import * as d from "./d"; +import * as e from "./e"; +import * as e2 from "./e.js"; +import * as f from "./f"; +import * as g from "./dir"; +console.log(a.A); +console.log(b.B); +console.log(c.C); +console.log(d.D); +console.log(e.E); +console.log(e2.E); +console.log(f.F); +console.log(g.G); diff --git a/tests/specs/run/sloppy_imports/no_sloppy.out b/tests/specs/run/sloppy_imports/no_sloppy.out new file mode 100644 index 00000000000000..d3a205e9908a36 --- /dev/null +++ b/tests/specs/run/sloppy_imports/no_sloppy.out @@ -0,0 +1,2 @@ +error: Module not found "file:///[WILDCARD]/a.js". Maybe change the extension to '.ts' or run with --unstable-sloppy-imports + at file:///[WILDLINE]/main.ts:1:20 diff --git a/tests/specs/run/sloppy_imports/sloppy.out b/tests/specs/run/sloppy_imports/sloppy.out new file mode 100644 index 00000000000000..170a4bb1674a61 --- /dev/null +++ b/tests/specs/run/sloppy_imports/sloppy.out @@ -0,0 +1,8 @@ +[class A] +[class B] +[class C] +[class D] +[class E] +[class E] +[class F] +[class G] diff --git a/tests/specs/task/invalid_unstable_feature/invalid_unstable_feature.out b/tests/specs/task/invalid_unstable_feature/invalid_unstable_feature.out index 78c4eaca9fa735..901eea47a120cd 100644 --- a/tests/specs/task/invalid_unstable_feature/invalid_unstable_feature.out +++ b/tests/specs/task/invalid_unstable_feature/invalid_unstable_feature.out @@ -1,5 +1,4 @@ Task start deno run index.js -Warning Sloppy imports are not recommended and have a negative impact on performance. Warning 'abc' isn't a valid unstable feature Warning 'cba' isn't a valid unstable feature Hello unstable features diff --git a/tests/util/server/src/lsp.rs b/tests/util/server/src/lsp.rs index f210356105a456..b615ed475025aa 100644 --- a/tests/util/server/src/lsp.rs +++ b/tests/util/server/src/lsp.rs @@ -1230,6 +1230,16 @@ impl CollectedDiagnostics { .collect() } + pub fn for_file(&self, specifier: &Url) -> Vec { + self + .all_messages() + .iter() + .filter(|p| p.uri == *specifier) + .flat_map(|p| p.diagnostics.iter()) + .cloned() + .collect() + } + /// Gets the messages that the editor will see after all the publishes. pub fn all_messages(&self) -> Vec { self.0.clone() @@ -1245,7 +1255,7 @@ impl CollectedDiagnostics { .find(|p| { p.diagnostics .iter() - .any(|d| d.source == Some(source.to_string())) + .any(|d| d.source.as_deref() == Some(source)) }) .map(ToOwned::to_owned) .unwrap()