Skip to content

fix(check): ignore errors on ambient modules #29135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ use crate::args::deno_json::TsConfigResolver;
use crate::args::jsr_url;
use crate::args::CliLockfile;
use crate::args::CliOptions;
use crate::args::DenoSubcommand;
pub use crate::args::NpmCachingStrategy;
use crate::cache;
use crate::cache::FetchCacher;
Expand Down Expand Up @@ -79,6 +80,7 @@ pub struct GraphValidOptions<'a> {
/// Otherwise, surfaces integrity errors as errors.
pub exit_integrity_errors: bool,
pub allow_unknown_media_types: bool,
pub ignore_graph_errors: bool,
}

/// Check if `roots` and their deps are available. Returns `Ok(())` if
Expand Down Expand Up @@ -106,6 +108,7 @@ pub fn graph_valid(
check_js: options.check_js,
kind: options.kind,
allow_unknown_media_types: options.allow_unknown_media_types,
ignore_graph_errors: options.ignore_graph_errors,
},
);
if let Some(error) = errors.next() {
Expand Down Expand Up @@ -145,6 +148,7 @@ pub fn fill_graph_from_lockfile(
pub struct GraphWalkErrorsOptions<'a> {
pub check_js: CheckJsOption<'a>,
pub kind: GraphKind,
pub ignore_graph_errors: bool,
pub allow_unknown_media_types: bool,
}

Expand All @@ -156,10 +160,55 @@ pub fn graph_walk_errors<'a>(
roots: &'a [ModuleSpecifier],
options: GraphWalkErrorsOptions<'a>,
) -> impl Iterator<Item = JsErrorBox> + 'a {
fn should_ignore_resolution_error_for_types(err: &ResolutionError) -> bool {
match err {
ResolutionError::InvalidSpecifier { .. } => true,
ResolutionError::ResolverError { error, .. } => match error.as_ref() {
ResolveError::Specifier(_) => true,
ResolveError::ImportMap(err) => match err.as_kind() {
import_map::ImportMapErrorKind::UnmappedBareSpecifier(_, _) => true,
import_map::ImportMapErrorKind::JsonParse(_)
| import_map::ImportMapErrorKind::ImportMapNotObject
| import_map::ImportMapErrorKind::ImportsFieldNotObject
| import_map::ImportMapErrorKind::ScopesFieldNotObject
| import_map::ImportMapErrorKind::ScopePrefixNotObject(_)
| import_map::ImportMapErrorKind::BlockedByNullEntry(_)
| import_map::ImportMapErrorKind::SpecifierResolutionFailure {
..
}
| import_map::ImportMapErrorKind::SpecifierBacktracksAbovePrefix {
..
} => false,
},
ResolveError::Other(_) => false,
},
ResolutionError::InvalidDowngrade { .. }
| ResolutionError::InvalidJsrHttpsTypesImport { .. }
| ResolutionError::InvalidLocalImport { .. } => false,
}
}

fn should_ignore_module_graph_error_for_types(
err: &ModuleGraphError,
) -> bool {
match err {
ModuleGraphError::ResolutionError(err) => {
should_ignore_resolution_error_for_types(err)
}
ModuleGraphError::TypesResolutionError(err) => {
should_ignore_resolution_error_for_types(err)
}
ModuleGraphError::ModuleError(module_error) => {
matches!(module_error, ModuleError::Missing { .. })
}
}
}

fn should_ignore_error(
sys: &CliSys,
graph_kind: GraphKind,
allow_unknown_media_types: bool,
ignore_graph_errors: bool,
error: &ModuleGraphError,
) -> bool {
if (graph_kind == GraphKind::TypesOnly || allow_unknown_media_types)
Expand All @@ -171,6 +220,11 @@ pub fn graph_walk_errors<'a>(
return true;
}

if ignore_graph_errors && should_ignore_module_graph_error_for_types(error)
{
return true;
}

// surface these as typescript diagnostics instead
graph_kind.include_types()
&& has_module_graph_error_for_tsc_diagnostic(sys, error)
Expand All @@ -192,6 +246,7 @@ pub fn graph_walk_errors<'a>(
sys,
graph.graph_kind(),
options.allow_unknown_media_types,
options.ignore_graph_errors,
&error,
) {
log::debug!("Ignoring: {}", error);
Expand Down Expand Up @@ -964,6 +1019,10 @@ impl ModuleGraphBuilder {
check_js: CheckJsOption::Custom(self.tsconfig_resolver.as_ref()),
exit_integrity_errors: true,
allow_unknown_media_types,
ignore_graph_errors: !matches!(
self.cli_options.sub_command(),
DenoSubcommand::Install { .. }
),
},
)
}
Expand Down
16 changes: 3 additions & 13 deletions cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ use crate::tools::lint::CliLinter;
use crate::tools::lint::CliLinterOptions;
use crate::tools::lint::LintRuleProvider;
use crate::tsc::DiagnosticCategory;
use crate::type_checker::ambient_modules_to_regex_string;
use crate::util::path::to_percent_decoded_str;

pub type ScopedAmbientModules = HashMap<Option<Arc<Url>>, MaybeAmbientModules>;
Expand Down Expand Up @@ -455,19 +456,8 @@ impl DeferredDiagnostics {
ambient.regex = None;
continue;
}
let mut regex_string = String::with_capacity(value.len() * 8);
regex_string.push('(');
let last = value.len() - 1;
for (idx, part) in value.into_iter().enumerate() {
let trimmed = part.trim_matches('"');
let escaped = regex::escape(trimmed);
let regex = escaped.replace("\\*", ".*");
regex_string.push_str(&regex);
if idx != last {
regex_string.push('|');
}
}
regex_string.push_str(")$");
let mut regex_string = ambient_modules_to_regex_string(&value);
regex_string.push('$');
if let Ok(regex) = regex::Regex::new(&regex_string).inspect_err(|e| {
lsp_warn!("failed to compile ambient modules pattern: {e} (pattern is {regex_string:?})");
}) {
Expand Down
1 change: 1 addition & 0 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ impl LanguageServer {
check_js: CheckJsOption::False,
exit_integrity_errors: false,
allow_unknown_media_types: true,
ignore_graph_errors: true,
},
)?;

Expand Down
1 change: 1 addition & 0 deletions cli/tools/doc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ pub async fn doc(
check_js: CheckJsOption::False,
kind: GraphKind::TypesOnly,
allow_unknown_media_types: false,
ignore_graph_errors: true,
},
);
for error in errors {
Expand Down
2 changes: 2 additions & 0 deletions cli/tsc/99_main_compiler.js
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,10 @@ function exec({ config, debug: debugFlag, rootNames, localOnly }) {

performanceProgram({ program });

const checker = program.getProgram().getTypeChecker();
ops.op_respond({
diagnostics: fromTypeScriptDiagnostics(diagnostics),
ambientModules: checker.getAmbientModules().map((symbol) => symbol.name),
stats: performanceEnd(),
});
debug("<<< exec stop");
Expand Down
6 changes: 6 additions & 0 deletions cli/tsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ pub struct Response {
pub diagnostics: Diagnostics,
/// If there was any build info associated with the exec request.
pub maybe_tsbuildinfo: Option<String>,
pub ambient_modules: Vec<String>,
/// Statistics from the check.
pub stats: Stats,
}
Expand Down Expand Up @@ -1207,8 +1208,10 @@ fn op_is_node_file(state: &mut OpState, #[string] path: &str) -> bool {
}

#[derive(Debug, Deserialize, Eq, PartialEq)]
#[serde(rename_all = "camelCase")]
struct RespondArgs {
pub diagnostics: Diagnostics,
pub ambient_modules: Vec<String>,
pub stats: Stats,
}

Expand Down Expand Up @@ -1447,11 +1450,13 @@ pub fn exec(

if let Some(response) = state.maybe_response {
let diagnostics = response.diagnostics;
let ambient_modules = response.ambient_modules;
let maybe_tsbuildinfo = state.maybe_tsbuildinfo;
let stats = response.stats;

Ok(Response {
diagnostics,
ambient_modules,
maybe_tsbuildinfo,
stats,
})
Expand Down Expand Up @@ -1770,6 +1775,7 @@ mod tests {
reports_unnecessary: None,
other: Default::default(),
}]),
ambient_modules: vec![],
stats: Stats(vec![("a".to_string(), 12)])
})
);
Expand Down
55 changes: 54 additions & 1 deletion cli/type_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -437,6 +437,24 @@ impl Iterator for DiagnosticsByFolderRealIterator<'_> {
}
}

/// Converts the list of ambient module names to regex string
pub fn ambient_modules_to_regex_string(ambient_modules: &[String]) -> String {
let mut regex_string = String::with_capacity(ambient_modules.len() * 8);
regex_string.push('(');
let last = ambient_modules.len() - 1;
for (idx, part) in ambient_modules.iter().enumerate() {
let trimmed = part.trim_matches('"');
let escaped = regex::escape(trimmed);
let regex = escaped.replace("\\*", ".*");
regex_string.push_str(&regex);
if idx != last {
regex_string.push('|');
}
}
regex_string.push(')');
regex_string
}

impl<'a> DiagnosticsByFolderRealIterator<'a> {
#[allow(clippy::too_many_arguments)]
fn check_diagnostics_in_folder(
Expand Down Expand Up @@ -546,11 +564,35 @@ impl<'a> DiagnosticsByFolderRealIterator<'a> {
code_cache,
)?;

let ambient_modules = response.ambient_modules;
log::debug!("Ambient Modules: {:?}", ambient_modules);

let ambient_modules_regex = if ambient_modules.is_empty() {
None
} else {
let mut regex_string = String::with_capacity(ambient_modules.len() * 8);
regex_string.push_str("^Cannot find module '");
regex_string.push_str(&ambient_modules_to_regex_string(&ambient_modules));
regex_string.push('\'');
regex::Regex::new(&regex_string)
.inspect_err(|e| {
log::warn!("Failed to create regex for ambient modules: {}", e);
})
.ok()
};

let mut response_diagnostics = response.diagnostics.filter(|d| {
self.should_include_diagnostic(self.options.type_check_mode, d)
});
response_diagnostics.apply_fast_check_source_maps(&self.graph);
let mut diagnostics = missing_diagnostics;
let mut diagnostics = missing_diagnostics.filter(|d| {
if let Some(ambient_modules_regex) = &ambient_modules_regex {
if let Some(message_text) = &d.message_text {
return !ambient_modules_regex.is_match(message_text);
}
}
true
});
diagnostics.extend(response_diagnostics);

if let Some(tsbuildinfo) = response.maybe_tsbuildinfo {
Expand Down Expand Up @@ -1018,6 +1060,7 @@ fn get_leading_comments(file_text: &str) -> Vec<String> {
mod test {
use deno_ast::MediaType;

use super::ambient_modules_to_regex_string;
use super::get_leading_comments;
use super::has_ts_check;

Expand Down Expand Up @@ -1063,4 +1106,14 @@ mod test {
"// ts-check\nconsole.log(5);"
));
}

#[test]
fn ambient_modules_to_regex_string_test() {
let result = ambient_modules_to_regex_string(&[
"foo".to_string(),
"*.css".to_string(),
"$virtual/module".to_string(),
]);
assert_eq!(result, r"(foo|.*\.css|\$virtual/module)");
}
}
18 changes: 18 additions & 0 deletions tests/specs/check/ambient_modules/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"steps": [{
"args": "check foo.ts",
"output": "foo.out"
}, {
"args": "check bar.ts",
"output": "bar.out",
"exitCode": 1
}, {
"args": "run foo.ts",
"output": "run.out",
"exitCode": 1
}, {
"args": "run bar.ts",
"output": "run.out",
"exitCode": 1
}]
}
5 changes: 5 additions & 0 deletions tests/specs/check/ambient_modules/bar.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Check file:///[WILDLINE]/ambient_modules/bar.ts
TS2307 [ERROR]: Cannot find module 'file:///logo.svg'.
at file:///[WILDLINE]/ambient_modules/bar.ts:1:18

error: Type checking failed.
2 changes: 2 additions & 0 deletions tests/specs/check/ambient_modules/bar.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import logo from "/logo.svg";
console.log(logo);
1 change: 1 addition & 0 deletions tests/specs/check/ambient_modules/foo.out
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Check file:///[WILDLINE]/ambient_modules/foo.ts
5 changes: 5 additions & 0 deletions tests/specs/check/ambient_modules/foo.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/// <reference types="./modules.d.ts" />
import logo from "/logo.svg";
import "./global.css";
import styles from "./styles.module.css";
console.log(logo, styles.classes);
9 changes: 9 additions & 0 deletions tests/specs/check/ambient_modules/modules.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
declare module "*.svg" {
const content: string;
export default content;
}
declare module "*.module.css" {
const classes: { readonly [key: string]: string };
export default classes;
}
declare module "*.css" {}
2 changes: 2 additions & 0 deletions tests/specs/check/ambient_modules/run.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
error: Module not found "file:///logo.svg".
at file:///[WILDLINE]/ambient_modules/[WILDLINE].ts:[WILDLINE]
Loading