From 66424032a2c78c6010c0a1a1b22a26d081166660 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 19 Feb 2024 10:28:41 -0500 Subject: [PATCH] feat(unstable/lint): no-slow-types for JSR packages (#22430) 1. Renames zap/fast-check to instead be a `no-slow-types` lint rule. 1. This lint rule is automatically run when doing `deno lint` for packages (deno.json files with a name, version, and exports field) 1. This lint rules still occurs on publish. It can be skipped by running with `--no-slow-types` --- Cargo.lock | 8 +- cli/Cargo.toml | 4 +- cli/args/flags.rs | 10 +- cli/graph_util.rs | 24 +- cli/lsp/diagnostics.rs | 6 +- cli/tools/{lint.rs => lint/mod.rs} | 410 +++++++++++++----- cli/tools/lint/no_slow_types.rs | 38 ++ cli/tools/registry/diagnostics.rs | 20 +- cli/tools/registry/graph.rs | 117 ----- cli/tools/registry/mod.rs | 195 ++++----- cli/tools/registry/publish_order.rs | 42 +- tests/integration/lint_tests.rs | 42 ++ tests/integration/publish_tests.rs | 16 +- tests/testdata/lint/no_slow_types/a.ts | 3 + tests/testdata/lint/no_slow_types/b.ts | 5 + tests/testdata/lint/no_slow_types/c.ts | 4 + tests/testdata/lint/no_slow_types/d.ts | 4 + tests/testdata/lint/no_slow_types/deno.json | 8 + .../lint/no_slow_types/deno.non-package.json | 2 + .../lint/no_slow_types/no_slow_types.out | 35 ++ .../no_slow_types_entrypoint.out | 35 ++ .../lint/no_slow_types_workspace/a/b.ts | 5 + .../lint/no_slow_types_workspace/a/d.ts | 4 + .../lint/no_slow_types_workspace/a/deno.json | 8 + .../lint/no_slow_types_workspace/a/mod.ts | 3 + .../lint/no_slow_types_workspace/b/deno.json | 5 + .../lint/no_slow_types_workspace/b/mod.ts | 4 + .../lint/no_slow_types_workspace/c/deno.json | 5 + .../lint/no_slow_types_workspace/c/mod_c.ts | 4 + .../lint/no_slow_types_workspace/deno.json | 7 + .../lint/no_slow_types_workspace/output.out | 46 ++ tests/testdata/publish/allow_slow_types.out | 4 + tests/testdata/publish/deno_jsonc.out | 3 +- tests/testdata/publish/dry_run.out | 3 +- tests/testdata/publish/has_slow_types.out | 21 + .../deno.json | 0 .../mod.ts | 0 tests/testdata/publish/invalid_fast_check.out | 16 - tests/testdata/publish/invalid_import.out | 3 +- tests/testdata/publish/invalid_path.out | 3 +- .../testdata/publish/javascript_decl_file.out | 3 +- .../publish/javascript_missing_decl_file.out | 14 +- tests/testdata/publish/no_zap.out | 5 - tests/testdata/publish/node_specifier.out | 3 +- tests/testdata/publish/successful.out | 3 +- tests/testdata/publish/symlink.out | 3 +- .../publish/unanalyzable_dynamic_import.out | 3 +- tests/testdata/publish/workspace.out | 3 +- .../testdata/publish/workspace_individual.out | 3 +- 49 files changed, 781 insertions(+), 431 deletions(-) rename cli/tools/{lint.rs => lint/mod.rs} (58%) create mode 100644 cli/tools/lint/no_slow_types.rs create mode 100644 tests/testdata/lint/no_slow_types/a.ts create mode 100644 tests/testdata/lint/no_slow_types/b.ts create mode 100644 tests/testdata/lint/no_slow_types/c.ts create mode 100644 tests/testdata/lint/no_slow_types/d.ts create mode 100644 tests/testdata/lint/no_slow_types/deno.json create mode 100644 tests/testdata/lint/no_slow_types/deno.non-package.json create mode 100644 tests/testdata/lint/no_slow_types/no_slow_types.out create mode 100644 tests/testdata/lint/no_slow_types/no_slow_types_entrypoint.out create mode 100644 tests/testdata/lint/no_slow_types_workspace/a/b.ts create mode 100644 tests/testdata/lint/no_slow_types_workspace/a/d.ts create mode 100644 tests/testdata/lint/no_slow_types_workspace/a/deno.json create mode 100644 tests/testdata/lint/no_slow_types_workspace/a/mod.ts create mode 100644 tests/testdata/lint/no_slow_types_workspace/b/deno.json create mode 100644 tests/testdata/lint/no_slow_types_workspace/b/mod.ts create mode 100644 tests/testdata/lint/no_slow_types_workspace/c/deno.json create mode 100644 tests/testdata/lint/no_slow_types_workspace/c/mod_c.ts create mode 100644 tests/testdata/lint/no_slow_types_workspace/deno.json create mode 100644 tests/testdata/lint/no_slow_types_workspace/output.out create mode 100644 tests/testdata/publish/allow_slow_types.out create mode 100644 tests/testdata/publish/has_slow_types.out rename tests/testdata/publish/{invalid_fast_check => has_slow_types}/deno.json (100%) rename tests/testdata/publish/{invalid_fast_check => has_slow_types}/mod.ts (100%) delete mode 100644 tests/testdata/publish/invalid_fast_check.out delete mode 100644 tests/testdata/publish/no_zap.out diff --git a/Cargo.lock b/Cargo.lock index f97bbf340d..673712362c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1215,9 +1215,9 @@ dependencies = [ [[package]] name = "deno_config" -version = "0.9.2" +version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e587768367b7b1e353407feccaf1fee9358f83ccd2d75ce405d59ec480172831" +checksum = "ba7641dd37ffcc1aeb06dff206a3bdd9e9a52f177f5edd43b734933174c38067" dependencies = [ "anyhow", "glob", @@ -1418,9 +1418,9 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.66.0" +version = "0.66.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8c67c7c05d70e43560b1dfa38ee385d2d0153ccd4ea16fdc6a706881fd60f3c5" +checksum = "e10efbd226fb00e97c04350051cbb025957b2de025117493ee5b9e53cc7e230f" dependencies = [ "anyhow", "async-trait", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 3499677084..a7d6780e20 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -64,11 +64,11 @@ winres.workspace = true [dependencies] deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] } deno_cache_dir = { workspace = true } -deno_config = "=0.9.2" +deno_config = "=0.10.0" deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] } deno_doc = { version = "=0.107.0", features = ["html"] } deno_emit = "=0.37.0" -deno_graph = "=0.66.0" +deno_graph = "=0.66.2" deno_lint = { version = "=0.56.0", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "=0.17.0" diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 3b6810a957..0ce5212965 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -301,7 +301,7 @@ pub struct VendorFlags { pub struct PublishFlags { pub token: Option, pub dry_run: bool, - pub no_zap: bool, + pub allow_slow_types: bool, } #[derive(Clone, Debug, Eq, PartialEq)] @@ -2389,9 +2389,9 @@ fn publish_subcommand() -> Command { .action(ArgAction::SetTrue), ) .arg( - Arg::new("no-zap") - .long("no-zap") - .help("Skip Zap compatibility validation") + Arg::new("allow-slow-types") + .long("allow-slow-types") + .help("Allow publishing with slow types") .action(ArgAction::SetTrue), ) }) @@ -3828,7 +3828,7 @@ fn publish_parse(flags: &mut Flags, matches: &mut ArgMatches) { flags.subcommand = DenoSubcommand::Publish(PublishFlags { token: matches.remove_one("token"), dry_run: matches.get_flag("dry-run"), - no_zap: matches.get_flag("no-zap"), + allow_slow_types: matches.get_flag("allow-slow-types"), }); } diff --git a/cli/graph_util.rs b/cli/graph_util.rs index b9027afa3f..380d82cbf7 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -23,6 +23,7 @@ use crate::util::sync::TaskQueue; use crate::util::sync::TaskQueuePermit; use deno_config::ConfigFile; +use deno_config::WorkspaceMemberConfig; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::custom_error; @@ -277,7 +278,7 @@ impl ModuleGraphBuilder { graph_kind: GraphKind, roots: Vec, loader: &mut dyn Loader, - ) -> Result { + ) -> Result { self .create_graph_with_options(CreateGraphOptions { is_dynamic: false, @@ -289,10 +290,29 @@ impl ModuleGraphBuilder { .await } + pub async fn create_publish_graph( + &self, + packages: &[WorkspaceMemberConfig], + ) -> Result { + let mut roots = Vec::new(); + for package in packages { + roots.extend(package.config_file.resolve_export_value_urls()?); + } + self + .create_graph_with_options(CreateGraphOptions { + is_dynamic: false, + graph_kind: deno_graph::GraphKind::All, + roots, + workspace_fast_check: true, + loader: None, + }) + .await + } + pub async fn create_graph_with_options( &self, options: CreateGraphOptions<'_>, - ) -> Result { + ) -> Result { let mut graph = ModuleGraph::new(options.graph_kind); self diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 0b3faa5512..e3e206a52c 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -796,7 +796,11 @@ fn generate_lint_diagnostics( let documents = snapshot .documents .documents(DocumentsFilter::OpenDiagnosable); - let lint_rules = get_configured_rules(lint_options.rules.clone()); + let lint_rules = get_configured_rules( + lint_options.rules.clone(), + config.config_file.as_ref(), + ) + .rules; let mut diagnostics_vec = Vec::new(); for document in documents { let settings = diff --git a/cli/tools/lint.rs b/cli/tools/lint/mod.rs similarity index 58% rename from cli/tools/lint.rs rename to cli/tools/lint/mod.rs index 32b47e453c..e4a88f91c4 100644 --- a/cli/tools/lint.rs +++ b/cli/tools/lint/mod.rs @@ -2,28 +2,19 @@ //! This module provides file linting utilities using //! [`deno_lint`](https://github.com/denoland/deno_lint). -use crate::args::Flags; -use crate::args::LintFlags; -use crate::args::LintOptions; -use crate::args::LintReporterKind; -use crate::args::LintRulesConfig; -use crate::colors; -use crate::factory::CliFactory; -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::fs::FileCollector; -use crate::util::path::is_script_ext; -use crate::util::sync::AtomicFlag; 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::glob::FilePatterns; use deno_core::anyhow::bail; use deno_core::error::generic_error; use deno_core::error::AnyError; +use deno_core::parking_lot::Mutex; use deno_core::serde_json; +use deno_graph::FastCheckDiagnostic; use deno_lint::diagnostic::LintDiagnostic; use deno_lint::linter::LintFileOptions; use deno_lint::linter::Linter; @@ -33,15 +24,32 @@ 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; use std::io::Read; use std::path::Path; use std::path::PathBuf; use std::sync::Arc; -use std::sync::Mutex; +use crate::args::Flags; +use crate::args::LintFlags; +use crate::args::LintOptions; +use crate::args::LintReporterKind; +use crate::args::LintRulesConfig; use crate::cache::IncrementalCache; +use crate::colors; +use crate::factory::CliFactory; +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::fs::FileCollector; +use crate::util::path::is_script_ext; +use crate::util::sync::AtomicFlag; + +pub mod no_slow_types; static STDIN_FILE_NAME: &str = "$deno$stdin.ts"; @@ -110,15 +118,18 @@ pub async fn lint(flags: Flags, lint_flags: LintFlags) -> Result<(), AnyError> { let success = if is_stdin { let reporter_kind = lint_options.reporter_kind; let reporter_lock = Arc::new(Mutex::new(create_reporter(reporter_kind))); - let lint_rules = get_config_rules_err_empty(lint_options.rules)?; + let lint_rules = get_config_rules_err_empty( + lint_options.rules, + cli_options.maybe_config_file().as_ref(), + )?; let file_path = cli_options.initial_cwd().join(STDIN_FILE_NAME); - let r = lint_stdin(&file_path, lint_rules); + let r = lint_stdin(&file_path, lint_rules.rules); let success = handle_lint_result( &file_path.to_string_lossy(), r, reporter_lock.clone(), ); - reporter_lock.lock().unwrap().close(1); + reporter_lock.lock().close(1); success } else { let target_files = @@ -146,61 +157,105 @@ async fn lint_files( paths: Vec, ) -> Result { let caches = factory.caches()?; - let lint_rules = get_config_rules_err_empty(lint_options.rules)?; + let maybe_config_file = factory.cli_options().maybe_config_file().as_ref(); + let lint_rules = + get_config_rules_err_empty(lint_options.rules, maybe_config_file)?; let incremental_cache = Arc::new(IncrementalCache::new( caches.lint_incremental_cache_db(), - // use a hash of the rule names in order to bust the cache - &{ - // ensure this is stable by sorting it - let mut names = lint_rules.iter().map(|r| r.code()).collect::>(); - names.sort_unstable(); - names - }, + &lint_rules.incremental_cache_state(), &paths, )); let target_files_len = paths.len(); let reporter_kind = lint_options.reporter_kind; + // todo(dsherret): abstract away this lock behind a performant interface let reporter_lock = Arc::new(Mutex::new(create_reporter(reporter_kind.clone()))); let has_error = Arc::new(AtomicFlag::default()); - run_parallelized(paths, { + let mut futures = Vec::with_capacity(2); + if lint_rules.no_slow_types { + if let Some(config_file) = maybe_config_file { + let members = config_file.to_workspace_members()?; + let has_error = has_error.clone(); + let reporter_lock = reporter_lock.clone(); + let module_graph_builder = factory.module_graph_builder().await?.clone(); + let path_urls = paths + .iter() + .filter_map(|p| ModuleSpecifier::from_file_path(p).ok()) + .collect::>(); + futures.push(deno_core::unsync::spawn(async move { + let graph = module_graph_builder.create_publish_graph(&members).await?; + // todo(dsherret): this isn't exactly correct as linting isn't properly + // setup to handle workspaces. Iterating over the workspace members + // should be done at a higher level because it also needs to take into + // account the config per workspace member. + for member in &members { + let export_urls = member.config_file.resolve_export_value_urls()?; + if !export_urls.iter().any(|url| path_urls.contains(url)) { + continue; // entrypoint is not specified, so skip + } + let diagnostics = no_slow_types::collect_no_slow_type_diagnostics( + &export_urls, + &graph, + ); + if !diagnostics.is_empty() { + has_error.raise(); + let mut reporter = reporter_lock.lock(); + for diagnostic in &diagnostics { + reporter + .visit_diagnostic(LintOrCliDiagnostic::FastCheck(diagnostic)); + } + } + } + Ok(()) + })); + } + } + + futures.push({ let has_error = has_error.clone(); - let lint_rules = lint_rules.clone(); + let lint_rules = lint_rules.rules.clone(); let reporter_lock = reporter_lock.clone(); let incremental_cache = incremental_cache.clone(); - move |file_path| { - let file_text = fs::read_to_string(&file_path)?; + deno_core::unsync::spawn(async move { + run_parallelized(paths, { + move |file_path| { + let file_text = 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(()); - } + // 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(()); + } - let r = lint_file(&file_path, file_text, lint_rules); - if let Ok((file_diagnostics, file_source)) = &r { - if file_diagnostics.is_empty() { - // update the incremental cache if there were no diagnostics - incremental_cache - .update_file(&file_path, file_source.text_info().text_str()) + let r = lint_file(&file_path, file_text, lint_rules); + if let Ok((file_diagnostics, file_source)) = &r { + if file_diagnostics.is_empty() { + // update the incremental cache if there were no diagnostics + incremental_cache + .update_file(&file_path, file_source.text_info().text_str()) + } + } + + let success = handle_lint_result( + &file_path.to_string_lossy(), + r, + reporter_lock.clone(), + ); + if !success { + has_error.raise(); + } + + Ok(()) } - } + }) + .await + }) + }); - let success = handle_lint_result( - &file_path.to_string_lossy(), - r, - reporter_lock.clone(), - ); - if !success { - has_error.raise(); - } + deno_core::futures::future::try_join_all(futures).await?; - Ok(()) - } - }) - .await?; incremental_cache.wait_completion().await; - reporter_lock.lock().unwrap().close(target_files_len); + reporter_lock.lock().close(target_files_len); Ok(!has_error.is_raised()) } @@ -311,16 +366,16 @@ fn handle_lint_result( result: Result<(Vec, ParsedSource), AnyError>, reporter_lock: Arc>>, ) -> bool { - let mut reporter = reporter_lock.lock().unwrap(); + let mut reporter = reporter_lock.lock(); match result { - Ok((mut file_diagnostics, source)) => { + Ok((mut file_diagnostics, _source)) => { file_diagnostics.sort_by(|a, b| match a.specifier.cmp(&b.specifier) { std::cmp::Ordering::Equal => a.range.start.cmp(&b.range.start), file_order => file_order, }); - for d in file_diagnostics.iter() { - reporter.visit_diagnostic(d, &source); + for d in &file_diagnostics { + reporter.visit_diagnostic(LintOrCliDiagnostic::Lint(d)); } file_diagnostics.is_empty() } @@ -331,8 +386,99 @@ 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: &LintDiagnostic, source: &ParsedSource); + fn visit_diagnostic(&mut self, d: LintOrCliDiagnostic); fn visit_error(&mut self, file_path: &str, err: &AnyError); fn close(&mut self, check_count: usize); } @@ -354,7 +500,7 @@ impl PrettyLintReporter { } impl LintReporter for PrettyLintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic, _source: &ParsedSource) { + fn visit_diagnostic(&mut self, d: LintOrCliDiagnostic) { self.lint_count += 1; eprintln!("{}", d.display()); @@ -391,18 +537,25 @@ impl CompactLintReporter { } impl LintReporter for CompactLintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic, _source: &ParsedSource) { + fn visit_diagnostic(&mut self, d: LintOrCliDiagnostic) { self.lint_count += 1; - let line_and_column = d.text_info.line_and_column_display(d.range.start); - eprintln!( - "{}: line {}, col {} - {} ({})", - d.specifier, - line_and_column.line_number, - line_and_column.column_number, - d.message, - d.code - ) + match d.range() { + Some((text_info, range)) => { + let line_and_column = text_info.line_and_column_display(range.start); + eprintln!( + "{}: line {}, col {} - {} ({})", + d.specifier(), + line_and_column.line_number, + line_and_column.column_number, + d.message(), + d.code(), + ) + } + None => { + eprintln!("{}: {} ({})", d.specifier(), d.message(), d.code()) + } + } } fn visit_error(&mut self, file_path: &str, err: &AnyError) { @@ -457,7 +610,7 @@ struct JsonLintDiagnosticRange { #[derive(Clone, Serialize)] struct JsonLintDiagnostic { pub filename: String, - pub range: JsonLintDiagnosticRange, + pub range: Option, pub message: String, pub code: String, pub hint: Option, @@ -479,22 +632,22 @@ impl JsonLintReporter { } impl LintReporter for JsonLintReporter { - fn visit_diagnostic(&mut self, d: &LintDiagnostic, _source: &ParsedSource) { + fn visit_diagnostic(&mut self, d: LintOrCliDiagnostic) { self.diagnostics.push(JsonLintDiagnostic { - filename: d.specifier.to_string(), - range: JsonLintDiagnosticRange { + filename: d.specifier().to_string(), + range: d.range().map(|(text_info, range)| JsonLintDiagnosticRange { start: JsonDiagnosticLintPosition::new( - d.range.start.as_byte_index(d.text_info.range().start), - d.text_info.line_and_column_index(d.range.start), + range.start.as_byte_index(text_info.range().start), + text_info.line_and_column_index(range.start), ), end: JsonDiagnosticLintPosition::new( - d.range.end.as_byte_index(d.text_info.range().start), - d.text_info.line_and_column_index(d.range.end), + range.end.as_byte_index(text_info.range().start), + text_info.line_and_column_index(range.end), ), - }, - message: d.message.clone(), - code: d.code.clone(), - hint: d.hint.clone(), + }), + message: d.message().to_string(), + code: d.code().to_string(), + hint: d.hint().map(|h| h.to_string()), }); } @@ -518,13 +671,22 @@ fn sort_diagnostics(diagnostics: &mut [JsonLintDiagnostic]) { use std::cmp::Ordering; let file_order = a.filename.cmp(&b.filename); match file_order { - Ordering::Equal => { - let line_order = a.range.start.line.cmp(&b.range.start.line); - match line_order { - Ordering::Equal => a.range.start.col.cmp(&b.range.start.col), - _ => line_order, - } - } + Ordering::Equal => match &a.range { + Some(a_range) => match &b.range { + Some(b_range) => { + let line_order = a_range.start.line.cmp(&b_range.start.line); + match line_order { + Ordering::Equal => a_range.start.col.cmp(&b_range.start.col), + _ => line_order, + } + } + None => Ordering::Less, + }, + None => match &b.range { + Some(_) => Ordering::Greater, + None => Ordering::Equal, + }, + }, _ => file_order, } }); @@ -532,26 +694,75 @@ fn sort_diagnostics(diagnostics: &mut [JsonLintDiagnostic]) { fn get_config_rules_err_empty( rules: LintRulesConfig, -) -> Result, AnyError> { - let lint_rules = get_configured_rules(rules); - if lint_rules.is_empty() { + maybe_config_file: Option<&deno_config::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 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, -) -> Vec<&'static dyn LintRule> { + maybe_config_file: Option<&deno_config::ConfigFile>, +) -> ConfiguredRules { + const NO_SLOW_TYPES_NAME: &str = "no-slow-types"; + let implicit_no_slow_types = maybe_config_file + .map(|c| c.is_package() || !c.json.workspaces.is_empty()) + .unwrap_or(false); if rules.tags.is_none() && rules.include.is_none() && rules.exclude.is_none() { - rules::get_recommended_rules() + ConfiguredRules { + rules: rules::get_recommended_rules(), + no_slow_types: implicit_no_slow_types, + } } else { - rules::get_filtered_rules( + 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(vec!["recommended".to_string()])), - rules.exclude, - rules.include, - ) + 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, + } } } @@ -569,8 +780,9 @@ mod test { include: None, tags: None, }; - let rules = get_configured_rules(rules_config); + let rules = get_configured_rules(rules_config, None); let mut rule_names = rules + .rules .into_iter() .map(|r| r.code().to_string()) .collect::>(); diff --git a/cli/tools/lint/no_slow_types.rs b/cli/tools/lint/no_slow_types.rs new file mode 100644 index 0000000000..6bb108a88d --- /dev/null +++ b/cli/tools/lint/no_slow_types.rs @@ -0,0 +1,38 @@ +// 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/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs index aeb5d61e2a..b605c293b0 100644 --- a/cli/tools/registry/diagnostics.rs +++ b/cli/tools/registry/diagnostics.rs @@ -30,7 +30,7 @@ pub struct PublishDiagnosticsCollector { impl PublishDiagnosticsCollector { pub fn print_and_error(&self) -> Result<(), AnyError> { let mut errors = 0; - let mut has_zap_errors = false; + let mut has_slow_types_errors = false; let diagnostics = self.diagnostics.lock().unwrap().take(); for diagnostic in diagnostics { eprint!("{}", diagnostic.display()); @@ -38,17 +38,23 @@ impl PublishDiagnosticsCollector { errors += 1; } if matches!(diagnostic, PublishDiagnostic::FastCheck(..)) { - has_zap_errors = true; + has_slow_types_errors = true; } } if errors > 0 { - if has_zap_errors { + if has_slow_types_errors { eprintln!( - "This package contains Zap errors. Although conforming to Zap will" + "This package contains errors for slow types. Fixing these errors will:\n" ); - eprintln!("significantly improve the type checking performance of your library,"); - eprintln!("you can choose to skip it by providing the --no-zap flag."); - eprintln!(); + eprintln!( + " 1. Significantly improve your package users' type checking performance." + ); + eprintln!(" 2. Improve the automatic documentation generation."); + eprintln!(" 3. Enable automatic .d.ts generation for Node.js."); + eprintln!( + "\nDon't want to bother? You can choose to skip this step by" + ); + eprintln!("providing the --allow-slow-types flag.\n"); } Err(anyhow!( diff --git a/cli/tools/registry/graph.rs b/cli/tools/registry/graph.rs index 3445d55e7c..0310a97c68 100644 --- a/cli/tools/registry/graph.rs +++ b/cli/tools/registry/graph.rs @@ -1,17 +1,9 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use std::collections::HashSet; -use std::collections::VecDeque; use std::sync::Arc; -use deno_ast::ModuleSpecifier; use deno_ast::SourceTextInfo; -use deno_config::ConfigFile; -use deno_config::WorkspaceConfig; -use deno_core::anyhow::bail; -use deno_core::anyhow::Context; -use deno_core::error::AnyError; -use deno_graph::FastCheckDiagnostic; use deno_graph::ModuleEntryRef; use deno_graph::ModuleGraph; use deno_graph::ResolutionResolved; @@ -21,55 +13,6 @@ use lsp_types::Url; use super::diagnostics::PublishDiagnostic; use super::diagnostics::PublishDiagnosticsCollector; -#[derive(Debug)] -pub struct MemberRoots { - pub name: String, - pub dir_url: ModuleSpecifier, - pub exports: Vec, -} - -pub fn get_workspace_member_roots( - config: &WorkspaceConfig, -) -> Result, AnyError> { - let mut members = Vec::with_capacity(config.members.len()); - let mut seen_names = HashSet::with_capacity(config.members.len()); - for member in &config.members { - if !seen_names.insert(&member.package_name) { - bail!( - "Cannot have two workspace packages with the same name ('{}' at {})", - member.package_name, - member.path.display(), - ); - } - members.push(MemberRoots { - name: member.package_name.clone(), - dir_url: member.config_file.specifier.join("./").unwrap().clone(), - exports: resolve_config_file_roots_from_exports(&member.config_file)?, - }); - } - Ok(members) -} - -pub fn resolve_config_file_roots_from_exports( - config_file: &ConfigFile, -) -> Result, AnyError> { - let exports_config = config_file - .to_exports_config() - .with_context(|| { - format!("Failed to parse exports at {}", config_file.specifier) - })? - .into_map(); - let mut exports = Vec::with_capacity(exports_config.len()); - for (_, value) in exports_config { - let entry_point = - config_file.specifier.join(&value).with_context(|| { - format!("Failed to join {} with {}", config_file.specifier, value) - })?; - exports.push(entry_point); - } - Ok(exports) -} - pub fn collect_invalid_external_imports( graph: &ModuleGraph, diagnostics_collector: &PublishDiagnosticsCollector, @@ -142,63 +85,3 @@ pub fn collect_invalid_external_imports( } } } - -/// Collects diagnostics from the module graph for the given packages. -/// Returns true if any diagnostics were collected. -pub fn collect_fast_check_type_graph_diagnostics( - graph: &ModuleGraph, - packages: &[MemberRoots], - diagnostics_collector: &PublishDiagnosticsCollector, -) -> bool { - let mut had_diagnostic = false; - let mut seen_modules = HashSet::with_capacity(graph.specifiers_count()); - for package in packages { - let mut pending = VecDeque::new(); - for export in &package.exports { - if seen_modules.insert(export.clone()) { - pending.push_back(export.clone()); - } - } - - 'analyze_package: while let Some(specifier) = pending.pop_front() { - let Ok(Some(module)) = graph.try_get_prefer_types(&specifier) else { - continue; - }; - let Some(es_module) = module.js() else { - continue; - }; - if let Some(diagnostics) = es_module.fast_check_diagnostics() { - for diagnostic in diagnostics { - had_diagnostic = true; - diagnostics_collector - .push(PublishDiagnostic::FastCheck(diagnostic.clone())); - if matches!( - diagnostic, - FastCheckDiagnostic::UnsupportedJavaScriptEntrypoint { .. } - ) { - break 'analyze_package; // no need to keep analyzing this package - } - } - } - - // analyze the next dependencies - for dep in es_module.dependencies_prefer_fast_check().values() { - let Some(specifier) = graph.resolve_dependency_from_dep(dep, true) - else { - continue; - }; - - let dep_in_same_package = - specifier.as_str().starts_with(package.dir_url.as_str()); - if dep_in_same_package { - let is_new = seen_modules.insert(specifier.clone()); - if is_new { - pending.push_back(specifier.clone()); - } - } - } - } - } - - had_diagnostic -} diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 9205d9b267..586115f27d 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use base64::prelude::BASE64_STANDARD; use base64::Engine; use deno_config::ConfigFile; +use deno_config::WorkspaceMemberConfig; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; @@ -33,12 +34,10 @@ use crate::factory::CliFactory; use crate::graph_util::ModuleGraphBuilder; use crate::http_util::HttpClient; use crate::tools::check::CheckOptions; +use crate::tools::lint::no_slow_types; +use crate::tools::registry::diagnostics::PublishDiagnostic; use crate::tools::registry::diagnostics::PublishDiagnosticsCollector; -use crate::tools::registry::graph::collect_fast_check_type_graph_diagnostics; use crate::tools::registry::graph::collect_invalid_external_imports; -use crate::tools::registry::graph::get_workspace_member_roots; -use crate::tools::registry::graph::resolve_config_file_roots_from_exports; -use crate::tools::registry::graph::MemberRoots; use crate::util::display::human_size; use crate::util::import_map::ImportMapUnfurler; @@ -80,16 +79,8 @@ impl PreparedPublishPackage { static SUGGESTED_ENTRYPOINTS: [&str; 4] = ["mod.ts", "mod.js", "index.ts", "index.js"]; -fn get_deno_json_package_name( - deno_json: &ConfigFile, -) -> Result { - match deno_json.json.name.clone() { - Some(name) => Ok(name), - None => bail!("{} is missing 'name' field", deno_json.specifier), - } -} - async fn prepare_publish( + package_name: &str, deno_json: &ConfigFile, source_cache: Arc, graph: Arc, @@ -101,7 +92,6 @@ async fn prepare_publish( let Some(version) = deno_json.json.version.clone() else { bail!("{} is missing 'version' field", deno_json.specifier); }; - let name = get_deno_json_package_name(deno_json)?; if deno_json.json.exports.is_none() { let mut suggested_entrypoint = None; @@ -118,22 +108,22 @@ async fn prepare_publish( "version": "{}", "exports": "{}" }}"#, - name, + package_name, version, suggested_entrypoint.unwrap_or("") ); bail!( "You did not specify an entrypoint to \"{}\" package in {}. Add `exports` mapping in the configuration file, eg:\n{}", - name, + package_name, deno_json.specifier, exports_content ); } - let Some(name) = name.strip_prefix('@') else { + let Some(name_no_at) = package_name.strip_prefix('@') else { bail!("Invalid package name, use '@/ format"); }; - let Some((scope, package_name)) = name.split_once('/') else { + let Some((scope, name_no_scope)) = name_no_at.split_once('/') else { bail!("Invalid package name, use '@/ format"); }; let file_patterns = deno_json.to_publish_config()?.map(|c| c.files); @@ -152,11 +142,11 @@ async fn prepare_publish( }) .await??; - log::debug!("Tarball size ({}): {}", name, tarball.bytes.len()); + log::debug!("Tarball size ({}): {}", package_name, tarball.bytes.len()); Ok(Rc::new(PreparedPublishPackage { scope: scope.to_string(), - package: package_name.to_string(), + package: name_no_scope.to_string(), version: version.to_string(), tarball, // the config file is always at the root of a publishing dir, @@ -660,77 +650,44 @@ struct PreparePackagesData { async fn prepare_packages_for_publishing( cli_factory: &CliFactory, - no_zap: bool, + allow_slow_types: bool, diagnostics_collector: &PublishDiagnosticsCollector, deno_json: ConfigFile, import_map: Arc, ) -> Result { - let maybe_workspace_config = deno_json.to_workspace_config()?; + let members = deno_json.to_workspace_members()?; let module_graph_builder = cli_factory.module_graph_builder().await?.as_ref(); let source_cache = cli_factory.parsed_source_cache(); let type_checker = cli_factory.type_checker().await?; let cli_options = cli_factory.cli_options(); - let Some(workspace_config) = maybe_workspace_config else { - let roots = resolve_config_file_roots_from_exports(&deno_json)?; - let graph = build_and_check_graph_for_publish( - module_graph_builder, - type_checker, - cli_options, - no_zap, - diagnostics_collector, - &[MemberRoots { - name: get_deno_json_package_name(&deno_json)?, - dir_url: deno_json.specifier.join("./").unwrap().clone(), - exports: roots, - }], - ) - .await?; - let package = prepare_publish( - &deno_json, - source_cache.clone(), - graph, - import_map, - diagnostics_collector, - ) - .await?; - let package_name = format!("@{}/{}", package.scope, package.package); - let publish_order_graph = - PublishOrderGraph::new_single(package_name.clone()); - let package_by_name = HashMap::from([(package_name, package)]); - return Ok(PreparePackagesData { - publish_order_graph, - package_by_name, - }); - }; + if members.len() > 1 { + println!("Publishing a workspace..."); + } - println!("Publishing a workspace..."); // create the module graph - let roots = get_workspace_member_roots(&workspace_config)?; let graph = build_and_check_graph_for_publish( module_graph_builder, type_checker, cli_options, - no_zap, + allow_slow_types, diagnostics_collector, - &roots, + &members, ) .await?; - let mut package_by_name = - HashMap::with_capacity(workspace_config.members.len()); + let mut package_by_name = HashMap::with_capacity(members.len()); let publish_order_graph = - publish_order::build_publish_order_graph(&graph, &roots)?; + publish_order::build_publish_order_graph(&graph, &members)?; - let results = workspace_config - .members - .iter() - .cloned() + let results = members + .into_iter() .map(|member| { let import_map = import_map.clone(); let graph = graph.clone(); async move { let package = prepare_publish( + &member.package_name, &member.config_file, source_cache.clone(), graph, @@ -761,64 +718,69 @@ async fn build_and_check_graph_for_publish( module_graph_builder: &ModuleGraphBuilder, type_checker: &TypeChecker, cli_options: &CliOptions, - no_zap: bool, + allow_slow_types: bool, diagnostics_collector: &PublishDiagnosticsCollector, - packages: &[MemberRoots], + packages: &[WorkspaceMemberConfig], ) -> Result, deno_core::anyhow::Error> { - let graph = Arc::new( - module_graph_builder - .create_graph_with_options(crate::graph_util::CreateGraphOptions { - is_dynamic: false, - // All because we're going to use this same graph to determine the publish order later - graph_kind: deno_graph::GraphKind::All, - roots: packages - .iter() - .flat_map(|r| r.exports.iter()) - .cloned() - .collect(), - workspace_fast_check: true, - loader: None, - }) - .await?, - ); + let graph = + Arc::new(module_graph_builder.create_publish_graph(packages).await?); graph.valid()?; + // todo(dsherret): move to lint rule collect_invalid_external_imports(&graph, diagnostics_collector); - let mut has_fast_check_diagnostics = false; - if !no_zap { - log::info!("Checking fast check type graph for errors..."); - has_fast_check_diagnostics = collect_fast_check_type_graph_diagnostics( - &graph, - packages, - diagnostics_collector, + if allow_slow_types { + log::info!( + concat!( + "{} Publishing a library with slow types is not recommended. ", + "This may lead to poor type checking performance for users of ", + "your package, may affect the quality of automatic documentation ", + "generation, and your package will not be shipped with a .d.ts ", + "file for Node.js users." + ), + colors::yellow("Warning"), ); - } + } else { + log::info!("Checking for slow types in the public API..."); + let mut any_pkg_had_diagnostics = false; + for package in packages { + let export_urls = package.config_file.resolve_export_value_urls()?; + let diagnostics = + no_slow_types::collect_no_slow_type_diagnostics(&export_urls, &graph); + if !diagnostics.is_empty() { + any_pkg_had_diagnostics = true; + for diagnostic in diagnostics { + diagnostics_collector.push(PublishDiagnostic::FastCheck(diagnostic)); + } + } + } - if !has_fast_check_diagnostics { - log::info!("Ensuring type checks..."); - let diagnostics = type_checker - .check_diagnostics( - graph.clone(), - CheckOptions { - lib: cli_options.ts_type_lib_window(), - log_ignored_options: false, - reload: cli_options.reload_flag(), - }, - ) - .await?; - if !diagnostics.is_empty() { - bail!( - concat!( - "{:#}\n\n", - "You may have discovered a bug in Deno's fast check implementation. ", - "Fast check is still early days and we would appreciate if you log a ", - "bug if you believe this is one: https://github.com/denoland/deno/issues/" - ), - diagnostics - ); + if !any_pkg_had_diagnostics { + // this is a temporary measure until we know that fast check is reliable and stable + let check_diagnostics = type_checker + .check_diagnostics( + graph.clone(), + CheckOptions { + lib: cli_options.ts_type_lib_window(), + log_ignored_options: false, + reload: cli_options.reload_flag(), + }, + ) + .await?; + if !check_diagnostics.is_empty() { + bail!( + concat!( + "Failed ensuring public API type output is valid.\n\n", + "{:#}\n\n", + "You may have discovered a bug in Deno. Please open an issue at: ", + "https://github.com/denoland/deno/issues/" + ), + check_diagnostics + ); + } } } + Ok(graph) } @@ -852,7 +814,7 @@ pub async fn publish( let prepared_data = prepare_packages_for_publishing( &cli_factory, - publish_flags.no_zap, + publish_flags.allow_slow_types, &diagnostics_collector, config_file.clone(), import_map, @@ -866,10 +828,7 @@ pub async fn publish( } if publish_flags.dry_run { - log::warn!( - "{} Aborting due to --dry-run", - crate::colors::yellow("Warning") - ); + log::warn!("{} Aborting due to --dry-run", colors::yellow("Warning")); return Ok(()); } diff --git a/cli/tools/registry/publish_order.rs b/cli/tools/registry/publish_order.rs index bb423b2b5c..ad0f72272b 100644 --- a/cli/tools/registry/publish_order.rs +++ b/cli/tools/registry/publish_order.rs @@ -4,12 +4,12 @@ use std::collections::HashMap; use std::collections::HashSet; use std::collections::VecDeque; +use deno_ast::ModuleSpecifier; +use deno_config::WorkspaceMemberConfig; use deno_core::anyhow::bail; use deno_core::error::AnyError; use deno_graph::ModuleGraph; -use super::graph::MemberRoots; - pub struct PublishOrderGraph { packages: HashMap>, in_degree: HashMap, @@ -17,14 +17,6 @@ pub struct PublishOrderGraph { } impl PublishOrderGraph { - pub fn new_single(package_name: String) -> Self { - Self { - packages: HashMap::from([(package_name.clone(), HashSet::new())]), - in_degree: HashMap::from([(package_name.clone(), 0)]), - reverse_map: HashMap::from([(package_name, Vec::new())]), - } - } - pub fn next(&mut self) -> Vec { let mut package_names_with_depth = self .in_degree @@ -122,22 +114,26 @@ impl PublishOrderGraph { pub fn build_publish_order_graph( graph: &ModuleGraph, - roots: &[MemberRoots], + roots: &[WorkspaceMemberConfig], ) -> Result { - let packages = build_pkg_deps(graph, roots); + let packages = build_pkg_deps(graph, roots)?; Ok(build_publish_order_graph_from_pkgs_deps(packages)) } fn build_pkg_deps( graph: &deno_graph::ModuleGraph, - roots: &[MemberRoots], -) -> HashMap> { + roots: &[WorkspaceMemberConfig], +) -> Result>, AnyError> { let mut members = HashMap::with_capacity(roots.len()); let mut seen_modules = HashSet::with_capacity(graph.modules().count()); - for root in roots { + let roots = roots + .iter() + .map(|r| (ModuleSpecifier::from_file_path(&r.dir_path).unwrap(), r)) + .collect::>(); + for (root_dir_url, root) in &roots { let mut deps = HashSet::new(); let mut pending = VecDeque::new(); - pending.extend(root.exports.clone()); + pending.extend(root.config_file.resolve_export_value_urls()?); while let Some(specifier) = pending.pop_front() { let Some(module) = graph.get(&specifier).and_then(|m| m.js()) else { continue; @@ -163,23 +159,23 @@ fn build_pkg_deps( if specifier.scheme() != "file" { continue; } - if specifier.as_str().starts_with(root.dir_url.as_str()) { + if specifier.as_str().starts_with(root_dir_url.as_str()) { if seen_modules.insert(specifier.clone()) { pending.push_back(specifier.clone()); } } else { - let found_root = roots - .iter() - .find(|root| specifier.as_str().starts_with(root.dir_url.as_str())); + let found_root = roots.iter().find(|(dir_url, _)| { + specifier.as_str().starts_with(dir_url.as_str()) + }); if let Some(root) = found_root { - deps.insert(root.name.clone()); + deps.insert(root.1.package_name.clone()); } } } } - members.insert(root.name.clone(), deps); + members.insert(root.package_name.clone(), deps); } - members + Ok(members) } fn build_publish_order_graph_from_pkgs_deps( diff --git a/tests/integration/lint_tests.rs b/tests/integration/lint_tests.rs index f7c9ead36e..ae04142625 100644 --- a/tests/integration/lint_tests.rs +++ b/tests/integration/lint_tests.rs @@ -210,3 +210,45 @@ fn lint_with_glob_config_and_flags() { assert_contains!(output, "Found 2 problems"); assert_contains!(output, "Checked 2 files"); } + +itest!(no_slow_types { + args: "lint", + output: "lint/no_slow_types/no_slow_types.out", + cwd: Some("lint/no_slow_types"), + exit_code: 1, +}); + +itest!(no_slow_types_entrypoint { + args: "lint a.ts", + output: "lint/no_slow_types/no_slow_types_entrypoint.out", + cwd: Some("lint/no_slow_types"), + exit_code: 1, +}); + +itest!(no_slow_types_non_entrypoint { + args: "lint d.ts", + output_str: Some("Checked 1 file\n"), + cwd: Some("lint/no_slow_types"), + exit_code: 0, +}); + +itest!(no_slow_types_excluded { + args: "lint --rules-exclude=no-slow-types", + output_str: Some("Checked 4 files\n"), + cwd: Some("lint/no_slow_types"), + exit_code: 0, +}); + +itest!(no_slow_types_non_package { + args: "lint --config=deno.non-package.json", + output_str: Some("Checked 4 files\n"), + cwd: Some("lint/no_slow_types"), + exit_code: 0, +}); + +itest!(no_slow_types_workspace { + args: "lint", + output: "lint/no_slow_types_workspace/output.out", + cwd: Some("lint/no_slow_types_workspace"), + exit_code: 1, +}); diff --git a/tests/integration/publish_tests.rs b/tests/integration/publish_tests.rs index b614005cce..48e62e9053 100644 --- a/tests/integration/publish_tests.rs +++ b/tests/integration/publish_tests.rs @@ -22,17 +22,17 @@ itest!(missing_deno_json { exit_code: 1, }); -itest!(invalid_fast_check { +itest!(has_slow_types { args: "publish --token 'sadfasdf'", - output: "publish/invalid_fast_check.out", - cwd: Some("publish/invalid_fast_check"), + output: "publish/has_slow_types.out", + cwd: Some("publish/has_slow_types"), exit_code: 1, }); -itest!(no_zap { - args: "publish --no-zap --token 'sadfasdf'", - output: "publish/no_zap.out", - cwd: Some("publish/invalid_fast_check"), +itest!(allow_slow_types { + args: "publish --allow-slow-types --token 'sadfasdf'", + output: "publish/allow_slow_types.out", + cwd: Some("publish/has_slow_types"), envs: env_vars_for_jsr_tests(), http_server: true, exit_code: 0, @@ -83,7 +83,9 @@ fn publish_non_exported_files_using_import_map() { .new_command() .args("publish --log-level=debug --token 'sadfasdf'") .run(); + output.assert_exit_code(0); let lines = output.combined_output().split('\n').collect::>(); + eprintln!("{}", output.combined_output()); assert!(lines .iter() .any(|l| l.contains("Unfurling") && l.ends_with("mod.ts"))); diff --git a/tests/testdata/lint/no_slow_types/a.ts b/tests/testdata/lint/no_slow_types/a.ts new file mode 100644 index 0000000000..3b399665dc --- /dev/null +++ b/tests/testdata/lint/no_slow_types/a.ts @@ -0,0 +1,3 @@ +export function add(a: number, b: number) { + return a + b; +} diff --git a/tests/testdata/lint/no_slow_types/b.ts b/tests/testdata/lint/no_slow_types/b.ts new file mode 100644 index 0000000000..b96a794894 --- /dev/null +++ b/tests/testdata/lint/no_slow_types/b.ts @@ -0,0 +1,5 @@ +export function addB(a: number, b: number) { + return a + b; +} + +export * from "./d.ts"; diff --git a/tests/testdata/lint/no_slow_types/c.ts b/tests/testdata/lint/no_slow_types/c.ts new file mode 100644 index 0000000000..517aa3d211 --- /dev/null +++ b/tests/testdata/lint/no_slow_types/c.ts @@ -0,0 +1,4 @@ +// this one won't error because it's not an export +export function addC(a: number, b: number) { + return a + b; +} diff --git a/tests/testdata/lint/no_slow_types/d.ts b/tests/testdata/lint/no_slow_types/d.ts new file mode 100644 index 0000000000..babe9d81b5 --- /dev/null +++ b/tests/testdata/lint/no_slow_types/d.ts @@ -0,0 +1,4 @@ +// this one is re-exported via b.ts +export function addD(a: number, b: number) { + return a + b; +} diff --git a/tests/testdata/lint/no_slow_types/deno.json b/tests/testdata/lint/no_slow_types/deno.json new file mode 100644 index 0000000000..2fd0af5f0f --- /dev/null +++ b/tests/testdata/lint/no_slow_types/deno.json @@ -0,0 +1,8 @@ +{ + "name": "@pkg/pkg", + "version": "1.0.0", + "exports": { + "./a": "./a.ts", + "./b": "./b.ts" + } +} diff --git a/tests/testdata/lint/no_slow_types/deno.non-package.json b/tests/testdata/lint/no_slow_types/deno.non-package.json new file mode 100644 index 0000000000..2c63c08510 --- /dev/null +++ b/tests/testdata/lint/no_slow_types/deno.non-package.json @@ -0,0 +1,2 @@ +{ +} diff --git a/tests/testdata/lint/no_slow_types/no_slow_types.out b/tests/testdata/lint/no_slow_types/no_slow_types.out new file mode 100644 index 0000000000..5828906e76 --- /dev/null +++ b/tests/testdata/lint/no_slow_types/no_slow_types.out @@ -0,0 +1,35 @@ +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]a.ts:1:17 + | +1 | export function add(a: number, b: number) { + | ^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]b.ts:1:17 + | +1 | export function addB(a: number, b: number) { + | ^^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]d.ts:2:17 + | +2 | export function addD(a: number, b: number) { + | ^^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +Found 3 problems +Checked 4 files diff --git a/tests/testdata/lint/no_slow_types/no_slow_types_entrypoint.out b/tests/testdata/lint/no_slow_types/no_slow_types_entrypoint.out new file mode 100644 index 0000000000..b8c1013bf0 --- /dev/null +++ b/tests/testdata/lint/no_slow_types/no_slow_types_entrypoint.out @@ -0,0 +1,35 @@ +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]a.ts:1:17 + | +1 | export function add(a: number, b: number) { + | ^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]b.ts:1:17 + | +1 | export function addB(a: number, b: number) { + | ^^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]d.ts:2:17 + | +2 | export function addD(a: number, b: number) { + | ^^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +Found 3 problems +Checked 1 file diff --git a/tests/testdata/lint/no_slow_types_workspace/a/b.ts b/tests/testdata/lint/no_slow_types_workspace/a/b.ts new file mode 100644 index 0000000000..b96a794894 --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/a/b.ts @@ -0,0 +1,5 @@ +export function addB(a: number, b: number) { + return a + b; +} + +export * from "./d.ts"; diff --git a/tests/testdata/lint/no_slow_types_workspace/a/d.ts b/tests/testdata/lint/no_slow_types_workspace/a/d.ts new file mode 100644 index 0000000000..babe9d81b5 --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/a/d.ts @@ -0,0 +1,4 @@ +// this one is re-exported via b.ts +export function addD(a: number, b: number) { + return a + b; +} diff --git a/tests/testdata/lint/no_slow_types_workspace/a/deno.json b/tests/testdata/lint/no_slow_types_workspace/a/deno.json new file mode 100644 index 0000000000..5a971cd85d --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/a/deno.json @@ -0,0 +1,8 @@ +{ + "name": "@pkg/a", + "version": "1.0.0", + "exports": { + "./a": "./mod.ts", + "./b": "./b.ts" + } +} diff --git a/tests/testdata/lint/no_slow_types_workspace/a/mod.ts b/tests/testdata/lint/no_slow_types_workspace/a/mod.ts new file mode 100644 index 0000000000..3b399665dc --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/a/mod.ts @@ -0,0 +1,3 @@ +export function add(a: number, b: number) { + return a + b; +} diff --git a/tests/testdata/lint/no_slow_types_workspace/b/deno.json b/tests/testdata/lint/no_slow_types_workspace/b/deno.json new file mode 100644 index 0000000000..c95aeb0290 --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/b/deno.json @@ -0,0 +1,5 @@ +{ + "name": "@pkg/b", + "version": "1.0.0", + "exports": "./mod.ts" +} diff --git a/tests/testdata/lint/no_slow_types_workspace/b/mod.ts b/tests/testdata/lint/no_slow_types_workspace/b/mod.ts new file mode 100644 index 0000000000..fa1c068def --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/b/mod.ts @@ -0,0 +1,4 @@ +// ok +export function addB(a: number, b: number): number { + return a + b; +} diff --git a/tests/testdata/lint/no_slow_types_workspace/c/deno.json b/tests/testdata/lint/no_slow_types_workspace/c/deno.json new file mode 100644 index 0000000000..36d6e2e671 --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/c/deno.json @@ -0,0 +1,5 @@ +{ + "name": "@pkg/c", + "version": "1.0.0", + "exports": "./mod_c.ts" +} diff --git a/tests/testdata/lint/no_slow_types_workspace/c/mod_c.ts b/tests/testdata/lint/no_slow_types_workspace/c/mod_c.ts new file mode 100644 index 0000000000..632a90b655 --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/c/mod_c.ts @@ -0,0 +1,4 @@ +// not ok +export function addC(a: number, b: number) { + return a + b; +} diff --git a/tests/testdata/lint/no_slow_types_workspace/deno.json b/tests/testdata/lint/no_slow_types_workspace/deno.json new file mode 100644 index 0000000000..e3dd981e50 --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/deno.json @@ -0,0 +1,7 @@ +{ + "workspaces": [ + "./a", + "./b", + "./c" + ] +} diff --git a/tests/testdata/lint/no_slow_types_workspace/output.out b/tests/testdata/lint/no_slow_types_workspace/output.out new file mode 100644 index 0000000000..05f54099b3 --- /dev/null +++ b/tests/testdata/lint/no_slow_types_workspace/output.out @@ -0,0 +1,46 @@ +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]b.ts:1:17 + | +1 | export function addB(a: number, b: number) { + | ^^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]d.ts:2:17 + | +2 | export function addD(a: number, b: number) { + | ^^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]mod.ts:1:17 + | +1 | export function add(a: number, b: number) { + | ^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +error[no-slow-types]: missing explicit return type in the public API + --> [WILDCARD]mod_c.ts:2:17 + | +2 | export function addC(a: number, b: number) { + | ^^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + + +Found 4 problems +Checked 5 files diff --git a/tests/testdata/publish/allow_slow_types.out b/tests/testdata/publish/allow_slow_types.out new file mode 100644 index 0000000000..fe3788021c --- /dev/null +++ b/tests/testdata/publish/allow_slow_types.out @@ -0,0 +1,4 @@ +Warning Publishing a library with slow types is not recommended. This may lead to poor type checking performance for users of your package, may affect the quality of automatic documentation generation, and your package will not be shipped with a .d.ts file for Node.js users. +Publishing @foo/bar@1.1.0 ... +Successfully published @foo/bar@1.1.0 +Visit http://127.0.0.1:4250/@foo/bar@1.1.0 for details diff --git a/tests/testdata/publish/deno_jsonc.out b/tests/testdata/publish/deno_jsonc.out index 2d5e4ffea6..aae82c3393 100644 --- a/tests/testdata/publish/deno_jsonc.out +++ b/tests/testdata/publish/deno_jsonc.out @@ -1,5 +1,4 @@ -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check file:///[WILDCARD]/publish/deno_jsonc/mod.ts Publishing @foo/bar@1.0.0 ... Successfully published @foo/bar@1.0.0 diff --git a/tests/testdata/publish/dry_run.out b/tests/testdata/publish/dry_run.out index f9f4df72e9..1fa5932945 100644 --- a/tests/testdata/publish/dry_run.out +++ b/tests/testdata/publish/dry_run.out @@ -1,4 +1,3 @@ -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check [WILDCARD] Warning Aborting due to --dry-run diff --git a/tests/testdata/publish/has_slow_types.out b/tests/testdata/publish/has_slow_types.out new file mode 100644 index 0000000000..06e0421450 --- /dev/null +++ b/tests/testdata/publish/has_slow_types.out @@ -0,0 +1,21 @@ +Checking for slow types in the public API... +error[missing-explicit-return-type]: missing explicit return type in the public API + --> [WILDCARD]mod.ts:2:17 + | +2 | export function getRandom() { + | ^^^^^^^^^ this function is missing an explicit return type + = hint: add an explicit return type to the function + + info: all functions in the public API must have an explicit return type + docs: https://jsr.io/go/slow-type-missing-explicit-return-type + +This package contains errors for slow types. Fixing these errors will: + + 1. Significantly improve your package users' type checking performance. + 2. Improve the automatic documentation generation. + 3. Enable automatic .d.ts generation for Node.js. + +Don't want to bother? You can choose to skip this step by +providing the --allow-slow-types flag. + +error: Found 1 problem diff --git a/tests/testdata/publish/invalid_fast_check/deno.json b/tests/testdata/publish/has_slow_types/deno.json similarity index 100% rename from tests/testdata/publish/invalid_fast_check/deno.json rename to tests/testdata/publish/has_slow_types/deno.json diff --git a/tests/testdata/publish/invalid_fast_check/mod.ts b/tests/testdata/publish/has_slow_types/mod.ts similarity index 100% rename from tests/testdata/publish/invalid_fast_check/mod.ts rename to tests/testdata/publish/has_slow_types/mod.ts diff --git a/tests/testdata/publish/invalid_fast_check.out b/tests/testdata/publish/invalid_fast_check.out deleted file mode 100644 index d18be45fe0..0000000000 --- a/tests/testdata/publish/invalid_fast_check.out +++ /dev/null @@ -1,16 +0,0 @@ -Checking fast check type graph for errors... -error[zap-missing-explicit-return-type]: missing explicit return type in the public API - --> [WILDCARD]mod.ts:2:17 - | -2 | export function getRandom() { - | ^^^^^^^^^ this function is missing an explicit return type - = hint: add an explicit return type to the function - - info: all functions in the public API must have an explicit return type - docs: https://jsr.io/go/zap-missing-explicit-return-type - -This package contains Zap errors. Although conforming to Zap will -significantly improve the type checking performance of your library, -you can choose to skip it by providing the --no-zap flag. - -error: Found 1 problem diff --git a/tests/testdata/publish/invalid_import.out b/tests/testdata/publish/invalid_import.out index ca9d20eed0..f123a341b0 100644 --- a/tests/testdata/publish/invalid_import.out +++ b/tests/testdata/publish/invalid_import.out @@ -2,8 +2,7 @@ Download http://localhost:4545/welcome.ts Download http://localhost:4545/echo.ts Download http://localhost:4545/npm/registry/chalk Download http://localhost:4545/npm/registry/chalk/chalk-5.0.1.tgz -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check file://[WILDCARD]mod.ts error[invalid-external-import]: invalid import to a non-JSR 'http' specifier --> [WILDCARD]mod.ts:1:8 diff --git a/tests/testdata/publish/invalid_path.out b/tests/testdata/publish/invalid_path.out index cd3e92e0cc..bad1a64959 100644 --- a/tests/testdata/publish/invalid_path.out +++ b/tests/testdata/publish/invalid_path.out @@ -1,5 +1,4 @@ -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check file://[WILDCARD]mod.ts error[invalid-path]: package path must not contain whitespace (found ' ') --> [WILDCARD]path with spaces.txt diff --git a/tests/testdata/publish/javascript_decl_file.out b/tests/testdata/publish/javascript_decl_file.out index deb66eba0b..2eda47cb48 100644 --- a/tests/testdata/publish/javascript_decl_file.out +++ b/tests/testdata/publish/javascript_decl_file.out @@ -1,5 +1,4 @@ -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check file:///[WILDCARD]/javascript_decl_file/mod.js Publishing @foo/bar@1.0.0 ... Successfully published @foo/bar@1.0.0 diff --git a/tests/testdata/publish/javascript_missing_decl_file.out b/tests/testdata/publish/javascript_missing_decl_file.out index 557451b29e..08e92e320d 100644 --- a/tests/testdata/publish/javascript_missing_decl_file.out +++ b/tests/testdata/publish/javascript_missing_decl_file.out @@ -1,11 +1,19 @@ -Checking fast check type graph for errors... -warning[zap-unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoints +Checking for slow types in the public API... +warning[unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoint --> [WILDCARD]mod.js = hint: add a type declaration (d.ts) for the JavaScript module, or rewrite it to TypeScript info: JavaScript files with no corresponding declaration require type inference to be type checked info: fast check avoids type inference, so JavaScript entrypoints should be avoided - docs: https://jsr.io/go/zap-unsupported-javascript-entrypoint + docs: https://jsr.io/go/slow-type-unsupported-javascript-entrypoint + +warning[unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoint + --> [WILDCARD]other.js + = hint: add a type declaration (d.ts) for the JavaScript module, or rewrite it to TypeScript + + info: JavaScript files with no corresponding declaration require type inference to be type checked + info: fast check avoids type inference, so JavaScript entrypoints should be avoided + docs: https://jsr.io/go/slow-type-unsupported-javascript-entrypoint Publishing @foo/bar@1.0.0 ... Successfully published @foo/bar@1.0.0 diff --git a/tests/testdata/publish/no_zap.out b/tests/testdata/publish/no_zap.out deleted file mode 100644 index 109964903a..0000000000 --- a/tests/testdata/publish/no_zap.out +++ /dev/null @@ -1,5 +0,0 @@ -Ensuring type checks... -Check file:///[WILDCARD]/mod.ts -Publishing @foo/bar@1.1.0 ... -Successfully published @foo/bar@1.1.0 -Visit http://127.0.0.1:4250/@foo/bar@1.1.0 for details diff --git a/tests/testdata/publish/node_specifier.out b/tests/testdata/publish/node_specifier.out index 7acb5b5ba1..9ba10c75b7 100644 --- a/tests/testdata/publish/node_specifier.out +++ b/tests/testdata/publish/node_specifier.out @@ -1,5 +1,4 @@ -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Download http://localhost:4545/npm/registry/@types/node Download http://localhost:4545/npm/registry/@types/node/node-[WILDCARD].tgz Check file:///[WILDCARD]/publish/node_specifier/mod.ts diff --git a/tests/testdata/publish/successful.out b/tests/testdata/publish/successful.out index 7dbe16807b..1dd6168eb0 100644 --- a/tests/testdata/publish/successful.out +++ b/tests/testdata/publish/successful.out @@ -1,5 +1,4 @@ -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check file:///[WILDCARD]/publish/successful/mod.ts Publishing @foo/bar@1.0.0 ... Successfully published @foo/bar@1.0.0 diff --git a/tests/testdata/publish/symlink.out b/tests/testdata/publish/symlink.out index d226fa18e1..9524494210 100644 --- a/tests/testdata/publish/symlink.out +++ b/tests/testdata/publish/symlink.out @@ -1,5 +1,4 @@ -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check [WILDCARD]mod.ts warning[unsupported-file-type]: unsupported file type 'symlink' --> [WILDCARD]symlink diff --git a/tests/testdata/publish/unanalyzable_dynamic_import.out b/tests/testdata/publish/unanalyzable_dynamic_import.out index 3be7ece875..97306c0792 100644 --- a/tests/testdata/publish/unanalyzable_dynamic_import.out +++ b/tests/testdata/publish/unanalyzable_dynamic_import.out @@ -1,5 +1,4 @@ -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check file://[WILDCARD]/mod.ts warning[unanalyzable-dynamic-import]: unable to analyze dynamic import --> [WILDCARD]mod.ts:1:7 diff --git a/tests/testdata/publish/workspace.out b/tests/testdata/publish/workspace.out index 588c22bbc6..ceffc48cf8 100644 --- a/tests/testdata/publish/workspace.out +++ b/tests/testdata/publish/workspace.out @@ -1,6 +1,5 @@ Publishing a workspace... -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check file:///[WILDCARD]/workspace/foo/mod.ts Check file:///[WILDCARD]/workspace/bar/mod.ts Publishing @foo/bar@1.0.0 ... diff --git a/tests/testdata/publish/workspace_individual.out b/tests/testdata/publish/workspace_individual.out index 4eadb45af6..61fac206bc 100644 --- a/tests/testdata/publish/workspace_individual.out +++ b/tests/testdata/publish/workspace_individual.out @@ -1,5 +1,4 @@ -Checking fast check type graph for errors... -Ensuring type checks... +Checking for slow types in the public API... Check file:///[WILDCARD]/workspace/bar/mod.ts Publishing @foo/bar@1.0.0 ... Successfully published @foo/bar@1.0.0