From 2244204b86f4f1f2bb39156dec21b2adf10a3cd7 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 15 Feb 2024 18:50:51 -0500 Subject: [PATCH 01/12] feat(unstable/lint): no-slow-types --- Cargo.lock | 4 +- cli/Cargo.toml | 2 +- cli/args/flags.rs | 10 +- cli/graph_util.rs | 42 ++- cli/lsp/diagnostics.rs | 6 +- cli/tools/{lint.rs => lint/mod.rs} | 399 +++++++++++++++++++++------- cli/tools/lint/no_slow_types.rs | 73 +++++ cli/tools/registry/diagnostics.rs | 14 +- cli/tools/registry/graph.rs | 117 -------- cli/tools/registry/mod.rs | 190 ++++++------- cli/tools/registry/publish_order.rs | 42 ++- 11 files changed, 529 insertions(+), 370 deletions(-) rename cli/tools/{lint.rs => lint/mod.rs} (60%) create mode 100644 cli/tools/lint/no_slow_types.rs diff --git a/Cargo.lock b/Cargo.lock index aa07029b1a7f8b..9145c5393506b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1202,9 +1202,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", diff --git a/cli/Cargo.toml b/cli/Cargo.toml index ca00774ea2cc8c..4c278e1f2e3f90 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -64,7 +64,7 @@ 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.103.0", features = ["html"] } deno_emit = "=0.36.0" diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 59d74716705e73..b9d0752530015f 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)] @@ -2388,9 +2388,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 3633784b833516..e4f311a595f361 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -22,6 +22,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; @@ -263,7 +264,7 @@ impl ModuleGraphBuilder { graph_kind: GraphKind, roots: Vec, loader: &mut dyn Loader, - ) -> Result { + ) -> Result { self .create_graph_with_options(CreateGraphOptions { graph_kind, @@ -274,10 +275,28 @@ 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 { + 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 { enum MutLoaderRef<'a> { Borrowed(&'a mut dyn Loader), Owned(cache::FetchCacher), @@ -631,6 +650,25 @@ fn get_resolution_error_bare_specifier( } } +/// Gets the graphs for the given workspace members. +pub fn segment_graph_by_workspace_member( + graph: &ModuleGraph, + members: &[WorkspaceMemberConfig], +) -> Result, AnyError> { + if members.len() == 1 { + return Ok(vec![(members.first().unwrap().clone(), graph.clone())]); + } + + let mut result = Vec::with_capacity(members.len()); + for package in members { + result.push(( + package.clone(), + graph.segment(&package.config_file.resolve_export_value_urls()?), + )); + } + Ok(result) +} + #[derive(Debug)] struct GraphData { graph: Arc, diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 0b3faa5512e837..e3e206a52ca357 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 60% rename from cli/tools/lint.rs rename to cli/tools/lint/mod.rs index 32b47e453c69e1..bcf625d725f02a 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::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::graph_util::segment_graph_by_workspace_member; +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,95 @@ 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(); + futures.push(deno_core::unsync::spawn(async move { + let graph = module_graph_builder.create_publish_graph(&members).await?; + for (package, graph) in + segment_graph_by_workspace_member(&graph, &members)? + { + let result = no_slow_types::collect_no_slow_type_diagnostics( + &ModuleSpecifier::from_file_path(&package.dir_path).unwrap(), + &graph, + ); + if let no_slow_types::NoSlowTypesOutput::Fail(diagnostics) = result { + 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 +356,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 +376,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 +490,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 +527,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 +600,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 +622,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 +661,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 +684,74 @@ 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()).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 +769,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 00000000000000..f8d8c391fe4611 --- /dev/null +++ b/cli/tools/lint/no_slow_types.rs @@ -0,0 +1,73 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use std::collections::HashSet; +use std::collections::VecDeque; + +use deno_ast::ModuleSpecifier; +use deno_graph::FastCheckDiagnostic; +use deno_graph::ModuleGraph; + +#[derive(Debug, Clone)] +pub enum NoSlowTypesOutput { + Pass, + HasJsExport, + Fail(Vec), +} + +/// Collects diagnostics from the module graph for the given packages. +/// Returns true if any diagnostics were collected. +pub fn collect_no_slow_type_diagnostics( + root_dir: &ModuleSpecifier, + graph: &ModuleGraph, +) -> NoSlowTypesOutput { + let mut seen_modules = HashSet::with_capacity(graph.specifiers_count()); + let mut found_diagnostics = Vec::new(); + let mut pending = VecDeque::new(); + for export in &graph.roots { + if seen_modules.insert(export.clone()) { + pending.push_back(export.clone()); + } + } + + 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() { + if diagnostics.iter().any(|diagnostic| { + matches!( + diagnostic, + FastCheckDiagnostic::UnsupportedJavaScriptEntrypoint { .. } + ) + }) { + return NoSlowTypesOutput::HasJsExport; + } + found_diagnostics.extend(diagnostics.iter().cloned()); + } + + // 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(root_dir.as_str()); + if dep_in_same_package { + let is_new = seen_modules.insert(specifier.clone()); + if is_new { + pending.push_back(specifier.clone()); + } + } + } + } + + if found_diagnostics.is_empty() { + NoSlowTypesOutput::Pass + } else { + NoSlowTypesOutput::Fail(found_diagnostics) + } +} diff --git a/cli/tools/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs index aeb5d61e2a118a..2972e97a80a60e 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,16 +38,18 @@ 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. Although fixing these errors" + ); + eprintln!("will significantly improve the type checking performance of your library,"); + eprintln!( + "you can choose to skip it by running with --allow-slow-types" ); - eprintln!("significantly improve the type checking performance of your library,"); - eprintln!("you can choose to skip it by providing the --no-zap flag."); eprintln!(); } diff --git a/cli/tools/registry/graph.rs b/cli/tools/registry/graph.rs index 3445d55e7ca154..0310a97c68e8a2 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 dfd20f5715f5aa..59009ca0667d17 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -7,7 +7,9 @@ use std::sync::Arc; use base64::prelude::BASE64_STANDARD; use base64::Engine; +use deno_ast::ModuleSpecifier; use deno_config::ConfigFile; +use deno_config::WorkspaceMemberConfig; use deno_core::anyhow::bail; use deno_core::anyhow::Context; use deno_core::error::AnyError; @@ -30,15 +32,14 @@ use crate::args::PublishFlags; use crate::cache::LazyGraphSourceParser; use crate::cache::ParsedSourceCache; use crate::factory::CliFactory; +use crate::graph_util::segment_graph_by_workspace_member; 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 +81,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 +94,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 +110,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 +144,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 +652,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,63 +720,66 @@ 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 { - // 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 !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 !allow_slow_types { + log::info!("Linting public types..."); + // todo(dsherret): parallelize + for (package, graph) in segment_graph_by_workspace_member(&graph, packages)? + { + let output = no_slow_types::collect_no_slow_type_diagnostics( + &ModuleSpecifier::from_file_path(&package.dir_path).unwrap(), + &graph, ); + match output { + no_slow_types::NoSlowTypesOutput::Pass => { + // this is a temporary measure until we know that fast check is reliable and stable + let check_diagnostics = type_checker + .check_diagnostics( + graph.into(), + 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 fast output type checks for '{}'.\n", + "{:#}\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/" + ), + package.package_name, + check_diagnostics + ); + } + } + no_slow_types::NoSlowTypesOutput::HasJsExport => { + // ignore + } + no_slow_types::NoSlowTypesOutput::Fail(diagnostics) => { + for diagnostic in diagnostics { + diagnostics_collector + .push(PublishDiagnostic::FastCheck(diagnostic)); + } + } + } } } + Ok(graph) } @@ -851,7 +813,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, diff --git a/cli/tools/registry/publish_order.rs b/cli/tools/registry/publish_order.rs index bb423b2b5c8e8a..ad0f72272b83d4 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( From caf497e8f0b4195b580ce7818d09f91f85a2d041 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 15 Feb 2024 19:48:25 -0500 Subject: [PATCH 02/12] Add more tests, but still WIP --- cli/graph_util.rs | 19 ----- cli/tools/lint/mod.rs | 11 +-- cli/tools/lint/no_slow_types.rs | 81 +++++++------------ cli/tools/registry/diagnostics.rs | 2 +- cli/tools/registry/mod.rs | 24 +++--- tests/integration/lint_tests.rs | 7 ++ tests/integration/publish_tests.rs | 14 ++-- 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/config.out | 46 +++++++++++ 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 + .../testdata/lint/no_slow_types/no_config.out | 0 .../{no_zap.out => allow_slow_types.out} | 2 - tests/testdata/publish/deno_jsonc.out | 2 +- tests/testdata/publish/dry_run.out | 2 +- ...alid_fast_check.out => has_slow_types.out} | 8 +- .../deno.json | 0 .../mod.ts | 0 tests/testdata/publish/invalid_import.out | 2 +- tests/testdata/publish/invalid_path.out | 2 +- .../testdata/publish/javascript_decl_file.out | 2 +- .../publish/javascript_missing_decl_file.out | 2 +- tests/testdata/publish/node_specifier.out | 2 +- tests/testdata/publish/successful.out | 2 +- tests/testdata/publish/symlink.out | 2 +- .../publish/unanalyzable_dynamic_import.out | 2 +- tests/testdata/publish/workspace.out | 2 +- .../testdata/publish/workspace_individual.out | 2 +- 31 files changed, 143 insertions(+), 121 deletions(-) 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/config.out 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_config.out rename tests/testdata/publish/{no_zap.out => allow_slow_types.out} (68%) rename tests/testdata/publish/{invalid_fast_check.out => has_slow_types.out} (64%) 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%) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index e4f311a595f361..a3e2eb1663a84d 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -650,25 +650,6 @@ fn get_resolution_error_bare_specifier( } } -/// Gets the graphs for the given workspace members. -pub fn segment_graph_by_workspace_member( - graph: &ModuleGraph, - members: &[WorkspaceMemberConfig], -) -> Result, AnyError> { - if members.len() == 1 { - return Ok(vec![(members.first().unwrap().clone(), graph.clone())]); - } - - let mut result = Vec::with_capacity(members.len()); - for package in members { - result.push(( - package.clone(), - graph.segment(&package.config_file.resolve_export_value_urls()?), - )); - } - Ok(result) -} - #[derive(Debug)] struct GraphData { graph: Arc, diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index bcf625d725f02a..4895266a4883ea 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -40,7 +40,6 @@ use crate::args::LintRulesConfig; use crate::cache::IncrementalCache; use crate::colors; use crate::factory::CliFactory; -use crate::graph_util::segment_graph_by_workspace_member; use crate::tools::fmt::run_parallelized; use crate::util::file_watcher; use crate::util::fs::canonicalize_path; @@ -181,13 +180,9 @@ async fn lint_files( let module_graph_builder = factory.module_graph_builder().await?.clone(); futures.push(deno_core::unsync::spawn(async move { let graph = module_graph_builder.create_publish_graph(&members).await?; - for (package, graph) in - segment_graph_by_workspace_member(&graph, &members)? - { - let result = no_slow_types::collect_no_slow_type_diagnostics( - &ModuleSpecifier::from_file_path(&package.dir_path).unwrap(), - &graph, - ); + for member in &members { + let result = + no_slow_types::collect_no_slow_type_diagnostics(member, &graph)?; if let no_slow_types::NoSlowTypesOutput::Fail(diagnostics) = result { has_error.raise(); let mut reporter = reporter_lock.lock(); diff --git a/cli/tools/lint/no_slow_types.rs b/cli/tools/lint/no_slow_types.rs index f8d8c391fe4611..a24ca82ef0406d 100644 --- a/cli/tools/lint/no_slow_types.rs +++ b/cli/tools/lint/no_slow_types.rs @@ -1,9 +1,7 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use std::collections::HashSet; -use std::collections::VecDeque; - -use deno_ast::ModuleSpecifier; +use deno_config::WorkspaceMemberConfig; +use deno_core::error::AnyError; use deno_graph::FastCheckDiagnostic; use deno_graph::ModuleGraph; @@ -17,57 +15,34 @@ pub enum NoSlowTypesOutput { /// Collects diagnostics from the module graph for the given packages. /// Returns true if any diagnostics were collected. pub fn collect_no_slow_type_diagnostics( - root_dir: &ModuleSpecifier, + member: &WorkspaceMemberConfig, graph: &ModuleGraph, -) -> NoSlowTypesOutput { - let mut seen_modules = HashSet::with_capacity(graph.specifiers_count()); - let mut found_diagnostics = Vec::new(); - let mut pending = VecDeque::new(); - for export in &graph.roots { - if seen_modules.insert(export.clone()) { - pending.push_back(export.clone()); +) -> Result { + let export_urls = member.config_file.resolve_export_value_urls()?; + let mut js_exports = 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, so we only need to check the first one + let Some(module) = js_exports.next() else { + // could happen if all the exports are JSON + return Ok(NoSlowTypesOutput::Pass); + }; + + if let Some(diagnostics) = module.fast_check_diagnostics() { + if diagnostics.iter().any(|diagnostic| { + matches!( + diagnostic, + FastCheckDiagnostic::UnsupportedJavaScriptEntrypoint { .. } + ) + }) { + Ok(NoSlowTypesOutput::HasJsExport) + } else { + Ok(NoSlowTypesOutput::Fail( + diagnostics.iter().cloned().collect(), + )) } - } - - 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() { - if diagnostics.iter().any(|diagnostic| { - matches!( - diagnostic, - FastCheckDiagnostic::UnsupportedJavaScriptEntrypoint { .. } - ) - }) { - return NoSlowTypesOutput::HasJsExport; - } - found_diagnostics.extend(diagnostics.iter().cloned()); - } - - // 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(root_dir.as_str()); - if dep_in_same_package { - let is_new = seen_modules.insert(specifier.clone()); - if is_new { - pending.push_back(specifier.clone()); - } - } - } - } - - if found_diagnostics.is_empty() { - NoSlowTypesOutput::Pass } else { - NoSlowTypesOutput::Fail(found_diagnostics) + Ok(NoSlowTypesOutput::Pass) } } diff --git a/cli/tools/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs index 2972e97a80a60e..3f38e2d67a7efb 100644 --- a/cli/tools/registry/diagnostics.rs +++ b/cli/tools/registry/diagnostics.rs @@ -48,7 +48,7 @@ impl PublishDiagnosticsCollector { ); eprintln!("will significantly improve the type checking performance of your library,"); eprintln!( - "you can choose to skip it by running with --allow-slow-types" + "you can choose to skip it by providing the --allow-slow-types flag." ); eprintln!(); } diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 59009ca0667d17..7de24987f0d000 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -7,7 +7,6 @@ use std::sync::Arc; use base64::prelude::BASE64_STANDARD; use base64::Engine; -use deno_ast::ModuleSpecifier; use deno_config::ConfigFile; use deno_config::WorkspaceMemberConfig; use deno_core::anyhow::bail; @@ -32,7 +31,6 @@ use crate::args::PublishFlags; use crate::cache::LazyGraphSourceParser; use crate::cache::ParsedSourceCache; use crate::factory::CliFactory; -use crate::graph_util::segment_graph_by_workspace_member; use crate::graph_util::ModuleGraphBuilder; use crate::http_util::HttpClient; use crate::tools::check::CheckOptions; @@ -732,20 +730,16 @@ async fn build_and_check_graph_for_publish( collect_invalid_external_imports(&graph, diagnostics_collector); if !allow_slow_types { - log::info!("Linting public types..."); - // todo(dsherret): parallelize - for (package, graph) in segment_graph_by_workspace_member(&graph, packages)? - { - let output = no_slow_types::collect_no_slow_type_diagnostics( - &ModuleSpecifier::from_file_path(&package.dir_path).unwrap(), - &graph, - ); + log::info!("Checking for slow public types..."); + for package in packages { + let output = + no_slow_types::collect_no_slow_type_diagnostics(&package, &graph)?; match output { no_slow_types::NoSlowTypesOutput::Pass => { // this is a temporary measure until we know that fast check is reliable and stable let check_diagnostics = type_checker .check_diagnostics( - graph.into(), + graph.clone().into(), CheckOptions { lib: cli_options.ts_type_lib_window(), log_ignored_options: false, @@ -756,11 +750,11 @@ async fn build_and_check_graph_for_publish( if !check_diagnostics.is_empty() { bail!( concat!( - "Failed ensuring fast output type checks for '{}'.\n", + "Failed ensuring fast types are valid for '{}'.\n", "{:#}\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/" + "You may have discovered a bug in Deno's fast types implementation. ", + "Fast types is still early days and we would appreciate if you log a ", + "bug: https://github.com/denoland/deno/issues/" ), package.package_name, check_diagnostics diff --git a/tests/integration/lint_tests.rs b/tests/integration/lint_tests.rs index f7c9ead36e14ea..6271621f96a1bf 100644 --- a/tests/integration/lint_tests.rs +++ b/tests/integration/lint_tests.rs @@ -210,3 +210,10 @@ 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/config.out", + cwd: Some("lint/no_slow_types"), + exit_code: 1, +}); diff --git a/tests/integration/publish_tests.rs b/tests/integration/publish_tests.rs index b614005ccebbb7..e0012584b13603 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, 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 00000000000000..3b399665dc81da --- /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 00000000000000..b96a794894c109 --- /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 00000000000000..517aa3d2118283 --- /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/config.out b/tests/testdata/lint/no_slow_types/config.out new file mode 100644 index 00000000000000..d9b7d466fcea30 --- /dev/null +++ b/tests/testdata/lint/no_slow_types/config.out @@ -0,0 +1,46 @@ +error[no-slow-types]: missing explicit return type in the public API +< --> V:\deno\tests\testdata\lint\no_slow_types\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/zap-missing-explicit-return-type +< +< + V:\deno\tests\testdata\lint\no_slow_types\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/zap-missing-explicit-return-type +< +< + V:\deno\tests\testdata\lint\no_slow_types\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/zap-missing-explicit-return-type +< +< + V:\deno\tests\testdata\lint\no_slow_types\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/zap-missing-explicit-return-type +< +< + [WILDCARD]mod.ts:2:17 | @@ -9,8 +9,8 @@ error[zap-missing-explicit-return-type]: missing explicit return type in the pub 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. +This package contains errors for slow types. Although fixing these errors +will significantly improve the type checking performance of your library, +you can choose to skip it 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_import.out b/tests/testdata/publish/invalid_import.out index ca9d20eed0ad9c..640a28078e5dcf 100644 --- a/tests/testdata/publish/invalid_import.out +++ b/tests/testdata/publish/invalid_import.out @@ -2,7 +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... +Checking for slow public types... Ensuring type checks... Check file://[WILDCARD]mod.ts error[invalid-external-import]: invalid import to a non-JSR 'http' specifier diff --git a/tests/testdata/publish/invalid_path.out b/tests/testdata/publish/invalid_path.out index cd3e92e0cc593d..8883b03ac21888 100644 --- a/tests/testdata/publish/invalid_path.out +++ b/tests/testdata/publish/invalid_path.out @@ -1,4 +1,4 @@ -Checking fast check type graph for errors... +Checking for slow public types... Ensuring type checks... Check file://[WILDCARD]mod.ts error[invalid-path]: package path must not contain whitespace (found ' ') diff --git a/tests/testdata/publish/javascript_decl_file.out b/tests/testdata/publish/javascript_decl_file.out index deb66eba0be78c..47552cc097dae6 100644 --- a/tests/testdata/publish/javascript_decl_file.out +++ b/tests/testdata/publish/javascript_decl_file.out @@ -1,4 +1,4 @@ -Checking fast check type graph for errors... +Checking for slow public types... Ensuring type checks... Check file:///[WILDCARD]/javascript_decl_file/mod.js Publishing @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 557451b29ebf19..4ce7390da5d58b 100644 --- a/tests/testdata/publish/javascript_missing_decl_file.out +++ b/tests/testdata/publish/javascript_missing_decl_file.out @@ -1,4 +1,4 @@ -Checking fast check type graph for errors... +Checking for slow public types... warning[zap-unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoints --> [WILDCARD]mod.js = hint: add a type declaration (d.ts) for the JavaScript module, or rewrite it to TypeScript diff --git a/tests/testdata/publish/node_specifier.out b/tests/testdata/publish/node_specifier.out index 7acb5b5ba1dbd5..76d830b5e0678c 100644 --- a/tests/testdata/publish/node_specifier.out +++ b/tests/testdata/publish/node_specifier.out @@ -1,4 +1,4 @@ -Checking fast check type graph for errors... +Checking for slow public types... Ensuring type checks... Download http://localhost:4545/npm/registry/@types/node Download http://localhost:4545/npm/registry/@types/node/node-[WILDCARD].tgz diff --git a/tests/testdata/publish/successful.out b/tests/testdata/publish/successful.out index 7dbe16807be851..1a426f1d60aabe 100644 --- a/tests/testdata/publish/successful.out +++ b/tests/testdata/publish/successful.out @@ -1,4 +1,4 @@ -Checking fast check type graph for errors... +Checking for slow public types... Ensuring type checks... Check file:///[WILDCARD]/publish/successful/mod.ts Publishing @foo/bar@1.0.0 ... diff --git a/tests/testdata/publish/symlink.out b/tests/testdata/publish/symlink.out index d226fa18e11cb3..4beba8fdd9c8b2 100644 --- a/tests/testdata/publish/symlink.out +++ b/tests/testdata/publish/symlink.out @@ -1,4 +1,4 @@ -Checking fast check type graph for errors... +Checking for slow public types... Ensuring type checks... Check [WILDCARD]mod.ts warning[unsupported-file-type]: unsupported file type 'symlink' diff --git a/tests/testdata/publish/unanalyzable_dynamic_import.out b/tests/testdata/publish/unanalyzable_dynamic_import.out index 3be7ece875e6d2..95bd38c6c77e39 100644 --- a/tests/testdata/publish/unanalyzable_dynamic_import.out +++ b/tests/testdata/publish/unanalyzable_dynamic_import.out @@ -1,4 +1,4 @@ -Checking fast check type graph for errors... +Checking for slow public types... Ensuring type checks... Check file://[WILDCARD]/mod.ts warning[unanalyzable-dynamic-import]: unable to analyze dynamic import diff --git a/tests/testdata/publish/workspace.out b/tests/testdata/publish/workspace.out index 588c22bbc6b91a..89963dafcfea49 100644 --- a/tests/testdata/publish/workspace.out +++ b/tests/testdata/publish/workspace.out @@ -1,5 +1,5 @@ Publishing a workspace... -Checking fast check type graph for errors... +Checking for slow public types... Ensuring type checks... Check file:///[WILDCARD]/workspace/foo/mod.ts Check file:///[WILDCARD]/workspace/bar/mod.ts diff --git a/tests/testdata/publish/workspace_individual.out b/tests/testdata/publish/workspace_individual.out index 4eadb45af6213b..4599cc88704118 100644 --- a/tests/testdata/publish/workspace_individual.out +++ b/tests/testdata/publish/workspace_individual.out @@ -1,4 +1,4 @@ -Checking fast check type graph for errors... +Checking for slow public types... Ensuring type checks... Check file:///[WILDCARD]/workspace/bar/mod.ts Publishing @foo/bar@1.0.0 ... From 31fb36391a6126a61aa64dfd480cf6e4c34786ec Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 15 Feb 2024 21:30:32 -0500 Subject: [PATCH 03/12] Add more tests. --- Cargo.lock | 4 +- Cargo.toml | 3 + cli/Cargo.toml | 2 +- cli/tools/lint/mod.rs | 13 ++- cli/tools/lint/no_slow_types.rs | 31 +++----- cli/tools/registry/mod.rs | 43 +++++----- tests/integration/lint_tests.rs | 21 +++++ tests/integration/publish_tests.rs | 2 + tests/testdata/lint/no_slow_types/config.out | 79 ++++++++----------- .../testdata/lint/no_slow_types/no_config.out | 0 .../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/deno_jsonc.out | 1 - tests/testdata/publish/dry_run.out | 1 - tests/testdata/publish/has_slow_types.out | 4 +- tests/testdata/publish/invalid_import.out | 1 - tests/testdata/publish/invalid_path.out | 1 - .../testdata/publish/javascript_decl_file.out | 1 - .../publish/javascript_missing_decl_file.out | 12 ++- tests/testdata/publish/node_specifier.out | 1 - tests/testdata/publish/successful.out | 1 - tests/testdata/publish/symlink.out | 1 - .../publish/unanalyzable_dynamic_import.out | 1 - tests/testdata/publish/workspace.out | 1 - .../testdata/publish/workspace_individual.out | 1 - 33 files changed, 201 insertions(+), 115 deletions(-) delete mode 100644 tests/testdata/lint/no_slow_types/no_config.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 diff --git a/Cargo.lock b/Cargo.lock index 91d2780fc0376d..d9fa1c360eedf7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1417,9 +1417,7 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.66.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8c67c7c05d70e43560b1dfa38ee385d2d0153ccd4ea16fdc6a706881fd60f3c5" +version = "0.66.1" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 7b068643222c60..4ec7c333086d02 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -358,3 +358,6 @@ opt-level = 3 opt-level = 3 [profile.release.package.base64-simd] opt-level = 3 + +[patch.crates-io] +deno_graph = { path = "../deno_graph" } diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 86c6678cad39e6..a0cd8f562ab300 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -68,7 +68,7 @@ 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.1" deno_lint = { version = "=0.56.0", features = ["docs"] } deno_lockfile.workspace = true deno_npm = "=0.17.0" diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index 4895266a4883ea..83905707a779b8 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -180,10 +180,14 @@ async fn lint_files( let module_graph_builder = factory.module_graph_builder().await?.clone(); 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 result = + let diagnostics = no_slow_types::collect_no_slow_type_diagnostics(member, &graph)?; - if let no_slow_types::NoSlowTypesOutput::Fail(diagnostics) = result { + if !diagnostics.is_empty() { has_error.raise(); let mut reporter = reporter_lock.lock(); for diagnostic in &diagnostics { @@ -713,8 +717,9 @@ pub fn get_configured_rules( 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()).unwrap_or(false); + 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() { ConfiguredRules { diff --git a/cli/tools/lint/no_slow_types.rs b/cli/tools/lint/no_slow_types.rs index a24ca82ef0406d..7ab2c11cfe91ba 100644 --- a/cli/tools/lint/no_slow_types.rs +++ b/cli/tools/lint/no_slow_types.rs @@ -5,19 +5,12 @@ use deno_core::error::AnyError; use deno_graph::FastCheckDiagnostic; use deno_graph::ModuleGraph; -#[derive(Debug, Clone)] -pub enum NoSlowTypesOutput { - Pass, - HasJsExport, - Fail(Vec), -} - /// Collects diagnostics from the module graph for the given packages. /// Returns true if any diagnostics were collected. pub fn collect_no_slow_type_diagnostics( member: &WorkspaceMemberConfig, graph: &ModuleGraph, -) -> Result { +) -> Result, AnyError> { let export_urls = member.config_file.resolve_export_value_urls()?; let mut js_exports = export_urls .iter() @@ -26,23 +19,17 @@ pub fn collect_no_slow_type_diagnostics( // package, so we only need to check the first one let Some(module) = js_exports.next() else { // could happen if all the exports are JSON - return Ok(NoSlowTypesOutput::Pass); + return Ok(vec![]); }; if let Some(diagnostics) = module.fast_check_diagnostics() { - if diagnostics.iter().any(|diagnostic| { - matches!( - diagnostic, - FastCheckDiagnostic::UnsupportedJavaScriptEntrypoint { .. } - ) - }) { - Ok(NoSlowTypesOutput::HasJsExport) - } else { - Ok(NoSlowTypesOutput::Fail( - diagnostics.iter().cloned().collect(), - )) - } + // todo(https://github.com/denoland/deno_graph/issues/384): move to deno_graph + let mut diagnostics = diagnostics.iter().cloned().collect::>(); + diagnostics.sort_by_cached_key(|d| { + (d.specifier().clone(), d.range().map(|r| r.range.clone())) + }); + Ok(diagnostics) } else { - Ok(NoSlowTypesOutput::Pass) + Ok(Vec::new()) } } diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 7de24987f0d000..6ad0ba385611c5 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -732,23 +732,22 @@ async fn build_and_check_graph_for_publish( if !allow_slow_types { log::info!("Checking for slow public types..."); for package in packages { - let output = + let diagnostics = no_slow_types::collect_no_slow_type_diagnostics(&package, &graph)?; - match output { - no_slow_types::NoSlowTypesOutput::Pass => { - // this is a temporary measure until we know that fast check is reliable and stable - let check_diagnostics = type_checker - .check_diagnostics( - graph.clone().into(), - CheckOptions { - lib: cli_options.ts_type_lib_window(), - log_ignored_options: false, - reload: cli_options.reload_flag(), - }, - ) - .await?; - if !check_diagnostics.is_empty() { - bail!( + if diagnostics.is_empty() { + // this is a temporary measure until we know that fast check is reliable and stable + let check_diagnostics = type_checker + .check_diagnostics( + graph.clone().into(), + 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 fast types are valid for '{}'.\n", "{:#}\n\n", @@ -759,16 +758,10 @@ async fn build_and_check_graph_for_publish( package.package_name, check_diagnostics ); - } } - no_slow_types::NoSlowTypesOutput::HasJsExport => { - // ignore - } - no_slow_types::NoSlowTypesOutput::Fail(diagnostics) => { - for diagnostic in diagnostics { - diagnostics_collector - .push(PublishDiagnostic::FastCheck(diagnostic)); - } + } else { + for diagnostic in diagnostics { + diagnostics_collector.push(PublishDiagnostic::FastCheck(diagnostic)); } } } diff --git a/tests/integration/lint_tests.rs b/tests/integration/lint_tests.rs index 6271621f96a1bf..1fe97903b0efea 100644 --- a/tests/integration/lint_tests.rs +++ b/tests/integration/lint_tests.rs @@ -217,3 +217,24 @@ itest!(no_slow_types { cwd: Some("lint/no_slow_types"), exit_code: 1, }); + +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 e0012584b13603..48e62e9053ff5b 100644 --- a/tests/integration/publish_tests.rs +++ b/tests/integration/publish_tests.rs @@ -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/config.out b/tests/testdata/lint/no_slow_types/config.out index d9b7d466fcea30..5828906e76952b 100644 --- a/tests/testdata/lint/no_slow_types/config.out +++ b/tests/testdata/lint/no_slow_types/config.out @@ -1,46 +1,35 @@ error[no-slow-types]: missing explicit return type in the public API -< --> V:\deno\tests\testdata\lint\no_slow_types\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/zap-missing-explicit-return-type -< -< - V:\deno\tests\testdata\lint\no_slow_types\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/zap-missing-explicit-return-type -< -< - V:\deno\tests\testdata\lint\no_slow_types\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/zap-missing-explicit-return-type -< -< - V:\deno\tests\testdata\lint\no_slow_types\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/zap-missing-explicit-return-type -< -< - [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_config.out b/tests/testdata/lint/no_slow_types/no_config.out deleted file mode 100644 index e69de29bb2d1d6..00000000000000 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 00000000000000..b96a794894c109 --- /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 00000000000000..babe9d81b538a4 --- /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 00000000000000..5a971cd85d5413 --- /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 00000000000000..3b399665dc81da --- /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 00000000000000..c95aeb029081d0 --- /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 00000000000000..fa1c068deff539 --- /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 00000000000000..36d6e2e671af02 --- /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 00000000000000..632a90b655b54e --- /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 00000000000000..e3dd981e509ce4 --- /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 00000000000000..05f54099b33776 --- /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/deno_jsonc.out b/tests/testdata/publish/deno_jsonc.out index 6496259a33ed01..41ef43b927e11b 100644 --- a/tests/testdata/publish/deno_jsonc.out +++ b/tests/testdata/publish/deno_jsonc.out @@ -1,5 +1,4 @@ Checking for slow public types... -Ensuring type checks... 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 1fcb978808b4fb..d75cc3f1dc9ca1 100644 --- a/tests/testdata/publish/dry_run.out +++ b/tests/testdata/publish/dry_run.out @@ -1,4 +1,3 @@ Checking for slow public types... -Ensuring type checks... 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 index b2e1ebd81fe759..c406d8765aeebb 100644 --- a/tests/testdata/publish/has_slow_types.out +++ b/tests/testdata/publish/has_slow_types.out @@ -1,5 +1,5 @@ Checking for slow public types... -error[zap-missing-explicit-return-type]: missing explicit return type 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() { @@ -7,7 +7,7 @@ error[zap-missing-explicit-return-type]: missing explicit return type in the pub = 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 + docs: https://jsr.io/go/slow-type-missing-explicit-return-type This package contains errors for slow types. Although fixing these errors will significantly improve the type checking performance of your library, diff --git a/tests/testdata/publish/invalid_import.out b/tests/testdata/publish/invalid_import.out index 640a28078e5dcf..79a5e8e1b369a2 100644 --- a/tests/testdata/publish/invalid_import.out +++ b/tests/testdata/publish/invalid_import.out @@ -3,7 +3,6 @@ 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 for slow public types... -Ensuring type checks... 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 8883b03ac21888..88d6969d49cae0 100644 --- a/tests/testdata/publish/invalid_path.out +++ b/tests/testdata/publish/invalid_path.out @@ -1,5 +1,4 @@ Checking for slow public types... -Ensuring type checks... 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 47552cc097dae6..84318d640eae1c 100644 --- a/tests/testdata/publish/javascript_decl_file.out +++ b/tests/testdata/publish/javascript_decl_file.out @@ -1,5 +1,4 @@ Checking for slow public types... -Ensuring type checks... 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 4ce7390da5d58b..2d6b0d2c1aa058 100644 --- a/tests/testdata/publish/javascript_missing_decl_file.out +++ b/tests/testdata/publish/javascript_missing_decl_file.out @@ -1,11 +1,19 @@ Checking for slow public types... -warning[zap-unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoints +warning[unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoints --> [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 entrypoints + --> [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/node_specifier.out b/tests/testdata/publish/node_specifier.out index 76d830b5e0678c..e52b07e1ab7584 100644 --- a/tests/testdata/publish/node_specifier.out +++ b/tests/testdata/publish/node_specifier.out @@ -1,5 +1,4 @@ Checking for slow public types... -Ensuring type checks... 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 1a426f1d60aabe..30622c45228877 100644 --- a/tests/testdata/publish/successful.out +++ b/tests/testdata/publish/successful.out @@ -1,5 +1,4 @@ Checking for slow public types... -Ensuring type checks... 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 4beba8fdd9c8b2..e4c198d6ec6b01 100644 --- a/tests/testdata/publish/symlink.out +++ b/tests/testdata/publish/symlink.out @@ -1,5 +1,4 @@ Checking for slow public types... -Ensuring type checks... 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 95bd38c6c77e39..b385054a85312f 100644 --- a/tests/testdata/publish/unanalyzable_dynamic_import.out +++ b/tests/testdata/publish/unanalyzable_dynamic_import.out @@ -1,5 +1,4 @@ Checking for slow public types... -Ensuring type checks... 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 89963dafcfea49..9b7cd7e451726d 100644 --- a/tests/testdata/publish/workspace.out +++ b/tests/testdata/publish/workspace.out @@ -1,6 +1,5 @@ Publishing a workspace... Checking for slow public types... -Ensuring type checks... 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 4599cc88704118..cfe1458cadfdd4 100644 --- a/tests/testdata/publish/workspace_individual.out +++ b/tests/testdata/publish/workspace_individual.out @@ -1,5 +1,4 @@ Checking for slow public types... -Ensuring type checks... Check file:///[WILDCARD]/workspace/bar/mod.ts Publishing @foo/bar@1.0.0 ... Successfully published @foo/bar@1.0.0 From 40a0a54f08e08089fb658c49cf7881cc02358ecb Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 15 Feb 2024 21:32:01 -0500 Subject: [PATCH 04/12] lint --- cli/tools/lint/no_slow_types.rs | 7 ++++--- cli/tools/registry/mod.rs | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/cli/tools/lint/no_slow_types.rs b/cli/tools/lint/no_slow_types.rs index 7ab2c11cfe91ba..cec5e9f9ebe5f7 100644 --- a/cli/tools/lint/no_slow_types.rs +++ b/cli/tools/lint/no_slow_types.rs @@ -16,7 +16,8 @@ pub fn collect_no_slow_type_diagnostics( .iter() .filter_map(|url| graph.get(url).and_then(|m| m.js())); // fast check puts the same diagnostics in each entrypoint for the - // package, so we only need to check the first one + // 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 Ok(vec![]); @@ -24,9 +25,9 @@ pub fn collect_no_slow_type_diagnostics( if let Some(diagnostics) = module.fast_check_diagnostics() { // todo(https://github.com/denoland/deno_graph/issues/384): move to deno_graph - let mut diagnostics = diagnostics.iter().cloned().collect::>(); + let mut diagnostics = diagnostics.clone(); diagnostics.sort_by_cached_key(|d| { - (d.specifier().clone(), d.range().map(|r| r.range.clone())) + (d.specifier().clone(), d.range().map(|r| r.range)) }); Ok(diagnostics) } else { diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 6ad0ba385611c5..e647b4f3778d1b 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -733,12 +733,12 @@ async fn build_and_check_graph_for_publish( log::info!("Checking for slow public types..."); for package in packages { let diagnostics = - no_slow_types::collect_no_slow_type_diagnostics(&package, &graph)?; + no_slow_types::collect_no_slow_type_diagnostics(package, &graph)?; if diagnostics.is_empty() { // this is a temporary measure until we know that fast check is reliable and stable let check_diagnostics = type_checker .check_diagnostics( - graph.clone().into(), + graph.clone(), CheckOptions { lib: cli_options.ts_type_lib_window(), log_ignored_options: false, From b6724c98a63f63b69544e485c10f6997e6970e5c Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 15 Feb 2024 21:46:01 -0500 Subject: [PATCH 05/12] Do not run lint rule when entrypoint isn't specified --- cli/tools/lint/mod.rs | 15 ++++++-- cli/tools/lint/no_slow_types.rs | 20 +++++------ cli/tools/registry/mod.rs | 3 +- tests/integration/lint_tests.rs | 16 ++++++++- .../{config.out => no_slow_types.out} | 0 .../no_slow_types_entrypoint.out | 35 +++++++++++++++++++ 6 files changed, 74 insertions(+), 15 deletions(-) rename tests/testdata/lint/no_slow_types/{config.out => no_slow_types.out} (100%) create mode 100644 tests/testdata/lint/no_slow_types/no_slow_types_entrypoint.out diff --git a/cli/tools/lint/mod.rs b/cli/tools/lint/mod.rs index 83905707a779b8..e4a88f91c4f897 100644 --- a/cli/tools/lint/mod.rs +++ b/cli/tools/lint/mod.rs @@ -25,6 +25,7 @@ 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; @@ -178,6 +179,10 @@ async fn lint_files( 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 @@ -185,8 +190,14 @@ async fn lint_files( // 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 diagnostics = - no_slow_types::collect_no_slow_type_diagnostics(member, &graph)?; + 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(); diff --git a/cli/tools/lint/no_slow_types.rs b/cli/tools/lint/no_slow_types.rs index cec5e9f9ebe5f7..70bcd40990f85b 100644 --- a/cli/tools/lint/no_slow_types.rs +++ b/cli/tools/lint/no_slow_types.rs @@ -1,18 +1,16 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. -use deno_config::WorkspaceMemberConfig; -use deno_core::error::AnyError; +use deno_ast::ModuleSpecifier; use deno_graph::FastCheckDiagnostic; use deno_graph::ModuleGraph; -/// Collects diagnostics from the module graph for the given packages. -/// Returns true if any diagnostics were collected. +/// Collects diagnostics from the module graph for the +/// given package's export URLs. pub fn collect_no_slow_type_diagnostics( - member: &WorkspaceMemberConfig, + package_export_urls: &[ModuleSpecifier], graph: &ModuleGraph, -) -> Result, AnyError> { - let export_urls = member.config_file.resolve_export_value_urls()?; - let mut js_exports = export_urls +) -> 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 @@ -20,7 +18,7 @@ pub fn collect_no_slow_type_diagnostics( // the first one JS entrypoint let Some(module) = js_exports.next() else { // could happen if all the exports are JSON - return Ok(vec![]); + return vec![]; }; if let Some(diagnostics) = module.fast_check_diagnostics() { @@ -29,8 +27,8 @@ pub fn collect_no_slow_type_diagnostics( diagnostics.sort_by_cached_key(|d| { (d.specifier().clone(), d.range().map(|r| r.range)) }); - Ok(diagnostics) + diagnostics } else { - Ok(Vec::new()) + Vec::new() } } diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index e647b4f3778d1b..6afc4c8e752d9a 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -732,8 +732,9 @@ async fn build_and_check_graph_for_publish( if !allow_slow_types { log::info!("Checking for slow public types..."); for package in packages { + let export_urls = package.config_file.resolve_export_value_urls()?; let diagnostics = - no_slow_types::collect_no_slow_type_diagnostics(package, &graph)?; + no_slow_types::collect_no_slow_type_diagnostics(&export_urls, &graph); if diagnostics.is_empty() { // this is a temporary measure until we know that fast check is reliable and stable let check_diagnostics = type_checker diff --git a/tests/integration/lint_tests.rs b/tests/integration/lint_tests.rs index 1fe97903b0efea..ae04142625553a 100644 --- a/tests/integration/lint_tests.rs +++ b/tests/integration/lint_tests.rs @@ -213,11 +213,25 @@ fn lint_with_glob_config_and_flags() { itest!(no_slow_types { args: "lint", - output: "lint/no_slow_types/config.out", + 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"), diff --git a/tests/testdata/lint/no_slow_types/config.out b/tests/testdata/lint/no_slow_types/no_slow_types.out similarity index 100% rename from tests/testdata/lint/no_slow_types/config.out rename to tests/testdata/lint/no_slow_types/no_slow_types.out 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 00000000000000..b8c1013bf0d50e --- /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 From c4c78e3d20b98e3a29f5c8c0f8cc111c8bec0415 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 15 Feb 2024 21:48:23 -0500 Subject: [PATCH 06/12] Update message --- cli/tools/registry/mod.rs | 2 +- tests/testdata/publish/deno_jsonc.out | 2 +- tests/testdata/publish/dry_run.out | 2 +- tests/testdata/publish/has_slow_types.out | 2 +- tests/testdata/publish/invalid_import.out | 2 +- tests/testdata/publish/invalid_path.out | 2 +- tests/testdata/publish/javascript_decl_file.out | 2 +- tests/testdata/publish/javascript_missing_decl_file.out | 2 +- tests/testdata/publish/node_specifier.out | 2 +- tests/testdata/publish/successful.out | 2 +- tests/testdata/publish/symlink.out | 2 +- tests/testdata/publish/unanalyzable_dynamic_import.out | 2 +- tests/testdata/publish/workspace.out | 2 +- tests/testdata/publish/workspace_individual.out | 2 +- 14 files changed, 14 insertions(+), 14 deletions(-) diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 6afc4c8e752d9a..529dcd74893b50 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -730,7 +730,7 @@ async fn build_and_check_graph_for_publish( collect_invalid_external_imports(&graph, diagnostics_collector); if !allow_slow_types { - log::info!("Checking for slow public types..."); + log::info!("Checking for slow types in the public API..."); for package in packages { let export_urls = package.config_file.resolve_export_value_urls()?; let diagnostics = diff --git a/tests/testdata/publish/deno_jsonc.out b/tests/testdata/publish/deno_jsonc.out index 41ef43b927e11b..aae82c33938425 100644 --- a/tests/testdata/publish/deno_jsonc.out +++ b/tests/testdata/publish/deno_jsonc.out @@ -1,4 +1,4 @@ -Checking for slow public types... +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 d75cc3f1dc9ca1..1fa5932945e989 100644 --- a/tests/testdata/publish/dry_run.out +++ b/tests/testdata/publish/dry_run.out @@ -1,3 +1,3 @@ -Checking for slow public types... +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 index c406d8765aeebb..6cb57f070bc0d9 100644 --- a/tests/testdata/publish/has_slow_types.out +++ b/tests/testdata/publish/has_slow_types.out @@ -1,4 +1,4 @@ -Checking for slow public types... +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 | diff --git a/tests/testdata/publish/invalid_import.out b/tests/testdata/publish/invalid_import.out index 79a5e8e1b369a2..f123a341b0c37f 100644 --- a/tests/testdata/publish/invalid_import.out +++ b/tests/testdata/publish/invalid_import.out @@ -2,7 +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 for slow public types... +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 88d6969d49cae0..bad1a64959404c 100644 --- a/tests/testdata/publish/invalid_path.out +++ b/tests/testdata/publish/invalid_path.out @@ -1,4 +1,4 @@ -Checking for slow public types... +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 84318d640eae1c..2eda47cb481d57 100644 --- a/tests/testdata/publish/javascript_decl_file.out +++ b/tests/testdata/publish/javascript_decl_file.out @@ -1,4 +1,4 @@ -Checking for slow public types... +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 2d6b0d2c1aa058..bdaacfe98a571a 100644 --- a/tests/testdata/publish/javascript_missing_decl_file.out +++ b/tests/testdata/publish/javascript_missing_decl_file.out @@ -1,4 +1,4 @@ -Checking for slow public types... +Checking for slow types in the public API... warning[unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoints --> [WILDCARD]mod.js = hint: add a type declaration (d.ts) for the JavaScript module, or rewrite it to TypeScript diff --git a/tests/testdata/publish/node_specifier.out b/tests/testdata/publish/node_specifier.out index e52b07e1ab7584..9ba10c75b7c33d 100644 --- a/tests/testdata/publish/node_specifier.out +++ b/tests/testdata/publish/node_specifier.out @@ -1,4 +1,4 @@ -Checking for slow public types... +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 30622c45228877..1dd6168eb0f73c 100644 --- a/tests/testdata/publish/successful.out +++ b/tests/testdata/publish/successful.out @@ -1,4 +1,4 @@ -Checking for slow public types... +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 e4c198d6ec6b01..9524494210eda6 100644 --- a/tests/testdata/publish/symlink.out +++ b/tests/testdata/publish/symlink.out @@ -1,4 +1,4 @@ -Checking for slow public types... +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 b385054a85312f..97306c07924330 100644 --- a/tests/testdata/publish/unanalyzable_dynamic_import.out +++ b/tests/testdata/publish/unanalyzable_dynamic_import.out @@ -1,4 +1,4 @@ -Checking for slow public types... +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 9b7cd7e451726d..ceffc48cf8db2d 100644 --- a/tests/testdata/publish/workspace.out +++ b/tests/testdata/publish/workspace.out @@ -1,5 +1,5 @@ Publishing a workspace... -Checking for slow public types... +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 cfe1458cadfdd4..61fac206bc2ce1 100644 --- a/tests/testdata/publish/workspace_individual.out +++ b/tests/testdata/publish/workspace_individual.out @@ -1,4 +1,4 @@ -Checking for slow public types... +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 From fb9ad35e661f5f2826b140591cc2f7d127c1cc5f Mon Sep 17 00:00:00 2001 From: David Sherret Date: Thu, 15 Feb 2024 21:53:56 -0500 Subject: [PATCH 07/12] Fix to only type check once --- cli/tools/registry/mod.rs | 55 +++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 26 deletions(-) diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index 529dcd74893b50..f1a58c5833483c 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -731,41 +731,44 @@ async fn build_and_check_graph_for_publish( if !allow_slow_types { 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() { - // 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 fast types are valid for '{}'.\n", - "{:#}\n\n", - "You may have discovered a bug in Deno's fast types implementation. ", - "Fast types is still early days and we would appreciate if you log a ", - "bug: https://github.com/denoland/deno/issues/" - ), - package.package_name, - check_diagnostics - ); - } - } else { + if !diagnostics.is_empty() { + any_pkg_had_diagnostics = true; for diagnostic in diagnostics { diagnostics_collector.push(PublishDiagnostic::FastCheck(diagnostic)); } } } + + 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 fast types are valid.\n", + "{:#}\n\n", + "You may have discovered a bug in Deno's fast types implementation. ", + "Fast types is still early days and we would appreciate if you log a ", + "bug: https://github.com/denoland/deno/issues/" + ), + check_diagnostics + ); + } + } } Ok(graph) From cd39505cae100d9c83aae0973ebd44502e14d453 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 16 Feb 2024 09:17:27 -0500 Subject: [PATCH 08/12] Update deno_graph --- Cargo.lock | 4 +++- Cargo.toml | 3 --- cli/Cargo.toml | 2 +- tests/testdata/publish/javascript_missing_decl_file.out | 4 ++-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d9fa1c360eedf7..5ff184e0649185 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1417,7 +1417,9 @@ dependencies = [ [[package]] name = "deno_graph" -version = "0.66.1" +version = "0.66.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e10efbd226fb00e97c04350051cbb025957b2de025117493ee5b9e53cc7e230f" dependencies = [ "anyhow", "async-trait", diff --git a/Cargo.toml b/Cargo.toml index 4ec7c333086d02..7b068643222c60 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -358,6 +358,3 @@ opt-level = 3 opt-level = 3 [profile.release.package.base64-simd] opt-level = 3 - -[patch.crates-io] -deno_graph = { path = "../deno_graph" } diff --git a/cli/Cargo.toml b/cli/Cargo.toml index a0cd8f562ab300..460785512bc57e 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -68,7 +68,7 @@ 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.1" +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/tests/testdata/publish/javascript_missing_decl_file.out b/tests/testdata/publish/javascript_missing_decl_file.out index bdaacfe98a571a..08e92e320d2c06 100644 --- a/tests/testdata/publish/javascript_missing_decl_file.out +++ b/tests/testdata/publish/javascript_missing_decl_file.out @@ -1,5 +1,5 @@ Checking for slow types in the public API... -warning[unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoints +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 @@ -7,7 +7,7 @@ warning[unsupported-javascript-entrypoint]: used a JavaScript module without typ info: fast check avoids type inference, so JavaScript entrypoints should be avoided docs: https://jsr.io/go/slow-type-unsupported-javascript-entrypoint -warning[unsupported-javascript-entrypoint]: used a JavaScript module without type declarations as an entrypoints +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 From f24efdc522ff90604e9bb4576e4fdea91e68f784 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 16 Feb 2024 09:28:24 -0500 Subject: [PATCH 09/12] Rework error message --- cli/tools/registry/mod.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/cli/tools/registry/mod.rs b/cli/tools/registry/mod.rs index f1a58c5833483c..a1fa561d634bd3 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -759,12 +759,11 @@ async fn build_and_check_graph_for_publish( if !check_diagnostics.is_empty() { bail!( concat!( - "Failed ensuring fast types are valid.\n", - "{:#}\n\n", - "You may have discovered a bug in Deno's fast types implementation. ", - "Fast types is still early days and we would appreciate if you log a ", - "bug: https://github.com/denoland/deno/issues/" - ), + "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 ); } From ce8dd07e01e750b79fe209251f18f046e2f0d894 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 16 Feb 2024 09:45:13 -0500 Subject: [PATCH 10/12] Remove out of date todo --- cli/tools/lint/no_slow_types.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/cli/tools/lint/no_slow_types.rs b/cli/tools/lint/no_slow_types.rs index 70bcd40990f85b..6bb108a88d6823 100644 --- a/cli/tools/lint/no_slow_types.rs +++ b/cli/tools/lint/no_slow_types.rs @@ -1,5 +1,6 @@ // 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; @@ -22,10 +23,13 @@ pub fn collect_no_slow_type_diagnostics( }; if let Some(diagnostics) = module.fast_check_diagnostics() { - // todo(https://github.com/denoland/deno_graph/issues/384): move to deno_graph let mut diagnostics = diagnostics.clone(); diagnostics.sort_by_cached_key(|d| { - (d.specifier().clone(), d.range().map(|r| r.range)) + ( + d.specifier().clone(), + d.range().map(|r| r.range), + d.code().to_string(), + ) }); diagnostics } else { From b4c3bf865e4a209db65c295f72eb3f63cd26ea40 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 16 Feb 2024 11:48:03 -0500 Subject: [PATCH 11/12] Add messages --- cli/tools/registry/diagnostics.rs | 12 ++++++++---- cli/tools/registry/mod.rs | 18 +++++++++++++----- tests/testdata/publish/allow_slow_types.out | 1 + tests/testdata/publish/has_slow_types.out | 11 ++++++++--- 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/cli/tools/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs index 3f38e2d67a7efb..67cd87a91b4178 100644 --- a/cli/tools/registry/diagnostics.rs +++ b/cli/tools/registry/diagnostics.rs @@ -44,13 +44,17 @@ impl PublishDiagnosticsCollector { if errors > 0 { if has_slow_types_errors { eprintln!( - "This package contains errors for slow types. Although fixing these errors" + "This package contains errors for slow types. Fixing these errors will:\n" ); - eprintln!("will significantly improve the type checking performance of your library,"); eprintln!( - "you can choose to skip it by providing the --allow-slow-types flag." + " 1. Significantly improve your package's type checking performance." ); - eprintln!(); + 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/mod.rs b/cli/tools/registry/mod.rs index a1fa561d634bd3..586115f27d5168 100644 --- a/cli/tools/registry/mod.rs +++ b/cli/tools/registry/mod.rs @@ -729,7 +729,18 @@ async fn build_and_check_graph_for_publish( // todo(dsherret): move to lint rule collect_invalid_external_imports(&graph, diagnostics_collector); - if !allow_slow_types { + 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 { @@ -817,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/tests/testdata/publish/allow_slow_types.out b/tests/testdata/publish/allow_slow_types.out index c7bf3ef7bedd83..fe3788021cc29e 100644 --- a/tests/testdata/publish/allow_slow_types.out +++ b/tests/testdata/publish/allow_slow_types.out @@ -1,3 +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/has_slow_types.out b/tests/testdata/publish/has_slow_types.out index 6cb57f070bc0d9..f2bdb8f9fc8adb 100644 --- a/tests/testdata/publish/has_slow_types.out +++ b/tests/testdata/publish/has_slow_types.out @@ -9,8 +9,13 @@ error[missing-explicit-return-type]: missing explicit return type in the public 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. Although fixing these errors -will significantly improve the type checking performance of your library, -you can choose to skip it by providing the --allow-slow-types flag. +This package contains errors for slow types. Fixing these errors will: + + 1. Significantly improve your package's 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 From e599a2bc67498131e7f7ee89363c07b57505777e Mon Sep 17 00:00:00 2001 From: David Sherret Date: Mon, 19 Feb 2024 09:39:35 -0500 Subject: [PATCH 12/12] Fix and update based on review --- cli/graph_util.rs | 1 + cli/tools/registry/diagnostics.rs | 2 +- tests/testdata/publish/has_slow_types.out | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/graph_util.rs b/cli/graph_util.rs index 1d10b06fd77989..380d82cbf75ce6 100644 --- a/cli/graph_util.rs +++ b/cli/graph_util.rs @@ -300,6 +300,7 @@ impl ModuleGraphBuilder { } self .create_graph_with_options(CreateGraphOptions { + is_dynamic: false, graph_kind: deno_graph::GraphKind::All, roots, workspace_fast_check: true, diff --git a/cli/tools/registry/diagnostics.rs b/cli/tools/registry/diagnostics.rs index 67cd87a91b4178..b605c293b0fab6 100644 --- a/cli/tools/registry/diagnostics.rs +++ b/cli/tools/registry/diagnostics.rs @@ -47,7 +47,7 @@ impl PublishDiagnosticsCollector { "This package contains errors for slow types. Fixing these errors will:\n" ); eprintln!( - " 1. Significantly improve your package's type checking performance." + " 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."); diff --git a/tests/testdata/publish/has_slow_types.out b/tests/testdata/publish/has_slow_types.out index f2bdb8f9fc8adb..06e04214508c97 100644 --- a/tests/testdata/publish/has_slow_types.out +++ b/tests/testdata/publish/has_slow_types.out @@ -11,7 +11,7 @@ error[missing-explicit-return-type]: missing explicit return type in the public This package contains errors for slow types. Fixing these errors will: - 1. Significantly improve your package's type checking performance. + 1. Significantly improve your package users' type checking performance. 2. Improve the automatic documentation generation. 3. Enable automatic .d.ts generation for Node.js.