Skip to content
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

fix(check): compiler options from workspace members #27785

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
95 commits
Select commit Hold shift + click to select a range
d52ccd2
feat(check): compiler options from workspace members
nayeemrmn Nov 22, 2024
88f7510
wildcard
nayeemrmn Nov 22, 2024
8f5465e
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Nov 22, 2024
9707974
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Nov 26, 2024
7a3a73c
fix file patterns
nayeemrmn Nov 26, 2024
27561dd
smart diagnostics concatenation
nayeemrmn Nov 26, 2024
b716050
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Nov 27, 2024
874260d
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Nov 27, 2024
6e3cedd
use deno_config temp branch
nayeemrmn Nov 27, 2024
3e9a63f
fix
nayeemrmn Nov 27, 2024
6b73819
fix fixture
nayeemrmn Nov 27, 2024
c521bfb
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Nov 29, 2024
14b3eb2
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Nov 30, 2024
6499ad6
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 4, 2024
de4e641
WorkspaceFileContainer
nayeemrmn Dec 5, 2024
b3bd581
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 5, 2024
871750a
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 6, 2024
f36d70d
move remote specifier handling to deno_config
nayeemrmn Dec 7, 2024
e33d51d
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 7, 2024
59511cb
cleanup
nayeemrmn Dec 7, 2024
7ceeb0b
fix --doc-only
nayeemrmn Dec 7, 2024
4d081d2
dedup discovered/specified config
nayeemrmn Dec 7, 2024
dab8352
use WorkspaceFileContainer in deno test
nayeemrmn Dec 9, 2024
655190e
fix ext flag
nayeemrmn Dec 9, 2024
c22b0e4
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 9, 2024
42eb554
fix fixture
nayeemrmn Dec 9, 2024
61aa023
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 10, 2024
0c44814
use WorkspaceFileContainer for test --watch
nayeemrmn Dec 10, 2024
78de1fb
use WorkspaceFileContainer for bench
nayeemrmn Dec 10, 2024
1af0cde
use single workspace dir for test -c and bench -c
nayeemrmn Dec 10, 2024
0c188eb
restore test --watch=... support
nayeemrmn Dec 10, 2024
b8b9eea
restore checking to test --watch
nayeemrmn Dec 10, 2024
37d45d0
use WorkspaceFileContainer for bench --watch
nayeemrmn Dec 10, 2024
83ee4c9
lint
nayeemrmn Dec 10, 2024
de37f41
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 10, 2024
f8be309
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 11, 2024
8f5d50c
private CliOptions::flags again
nayeemrmn Dec 11, 2024
0e2f9e7
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 11, 2024
f45d39b
move -c handling to CliOptions methods
nayeemrmn Dec 11, 2024
ff1f921
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 11, 2024
161468a
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 13, 2024
9a6263c
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 16, 2024
5d76fc9
remove need for outer factory
nayeemrmn Dec 16, 2024
5807b53
reduce dependency on outer CliOptions
nayeemrmn Dec 16, 2024
f5b258c
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 16, 2024
c7f2899
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 17, 2024
9f8d833
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 17, 2024
1c2fb5c
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 19, 2024
03cb973
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Dec 30, 2024
60de9fa
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 1, 2025
e91b0ef
fmt
nayeemrmn Jan 1, 2025
f61286b
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 6, 2025
01fb217
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 6, 2025
2eae9a9
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 6, 2025
b0cbae7
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 7, 2025
5a1bb6b
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 14, 2025
6128282
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 15, 2025
8719cb7
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 15, 2025
c8a0404
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 16, 2025
8d5fb5a
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 18, 2025
0ae3090
fix merge
nayeemrmn Jan 18, 2025
9d9a88f
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 20, 2025
766452f
single factory
nayeemrmn Jan 20, 2025
1e073ee
don't store specifier info
nayeemrmn Jan 20, 2025
1dd3614
remote CliFactoryWithWorkspaceFiles::initial_cwd()
nayeemrmn Jan 20, 2025
798f733
workspace_files
nayeemrmn Jan 20, 2025
36fcff8
coverage fixture
nayeemrmn Jan 20, 2025
971ec9b
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 20, 2025
1e9e24f
scoped jsx import source config
nayeemrmn Jan 20, 2025
c2a6fa1
Merge remote-tracking branch 'upstream/main' into check-workspace-mem…
nayeemrmn Jan 22, 2025
67c4da4
abstract CliOptions creation
nayeemrmn Jan 22, 2025
c885735
use closure for get_dirs_with_files
nayeemrmn Jan 22, 2025
85c8e4d
use closure for collect_specifiers
nayeemrmn Jan 22, 2025
532e3b3
CliFactor::with_watcher_communicator()
nayeemrmn Jan 22, 2025
bf82fe2
fixup! CliFactor::with_watcher_communicator()
nayeemrmn Jan 22, 2025
f8cb237
Merge branch 'main' into check-workspace-member-compiler-options
dsherret Jan 22, 2025
8ee8582
Update to mostly only change TypeChecker
dsherret Jan 22, 2025
a18bf43
Not sure why this test output was different?
dsherret Jan 22, 2025
7024cf8
clippy
dsherret Jan 22, 2025
209801a
improve finding members
dsherret Jan 23, 2025
58275aa
update to display diagnostics by folder
dsherret Jan 23, 2025
1b82b1c
Merge branch 'main' into check-workspace-member-compiler-options2
dsherret Jan 24, 2025
f830c67
Add TsConfigResolver
dsherret Jan 24, 2025
ebd5ca1
updates based on deno_config changes
dsherret Jan 24, 2025
857d993
only log once
dsherret Jan 24, 2025
b1013a9
conditionally inject @types/node
dsherret Jan 24, 2025
7339504
streaming errors and rework this to be more correct
dsherret Jan 25, 2025
64a9fe6
do not show check when there is nothing to check
dsherret Jan 25, 2025
40dd0a8
slightly better
dsherret Jan 25, 2025
bee6511
fix test
dsherret Jan 25, 2025
7255a9e
Merge branch 'main' into check-workspace-member-compiler-options2
dsherret Jan 27, 2025
4ab1c29
duplicates
dsherret Jan 27, 2025
9fcdfc7
Merge branch 'main' into check-workspace-member-compiler-options2
dsherret Jan 27, 2025
e16f70a
improve grouping files together for type checking
dsherret Jan 28, 2025
b1cda9d
Improve names
dsherret Jan 28, 2025
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
Next Next commit
feat(check): compiler options from workspace members
  • Loading branch information
nayeemrmn committed Nov 22, 2024
commit d52ccd2834a76596c5ec0be4dd656a9f7832fc4d
14 changes: 14 additions & 0 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ use once_cell::sync::Lazy;
use serde::Deserialize;
use serde::Serialize;
use std::borrow::Cow;
use std::collections::BTreeSet;
use std::collections::HashMap;
use std::env;
use std::io::BufReader;
Expand Down Expand Up @@ -790,6 +791,15 @@ struct CliOptionOverrides {
import_map_specifier: Option<Option<ModuleSpecifier>>,
}

/// Overrides for the options below that when set will
/// use these values over the values derived from the
/// CLI flags or config file.
#[derive(Debug, Clone)]
pub struct ScopeOptions {
pub scope: Option<Arc<ModuleSpecifier>>,
pub all_scopes: Arc<BTreeSet<Arc<ModuleSpecifier>>>,
}

/// Holds the resolved options of many sources used by subcommands
/// and provides some helper function for creating common objects.
pub struct CliOptions {
Expand All @@ -804,6 +814,7 @@ pub struct CliOptions {
overrides: CliOptionOverrides,
pub start_dir: Arc<WorkspaceDirectory>,
pub deno_dir_provider: Arc<DenoDirProvider>,
pub scope_options: Option<Arc<ScopeOptions>>,
}

impl CliOptions {
Expand All @@ -814,6 +825,7 @@ impl CliOptions {
npmrc: Arc<ResolvedNpmRc>,
start_dir: Arc<WorkspaceDirectory>,
force_global_cache: bool,
scope_options: Option<Arc<ScopeOptions>>,
) -> Result<Self, AnyError> {
if let Some(insecure_allowlist) =
flags.unsafely_ignore_certificate_errors.as_ref()
Expand Down Expand Up @@ -853,6 +865,7 @@ impl CliOptions {
main_module_cell: std::sync::OnceLock::new(),
start_dir,
deno_dir_provider,
scope_options,
})
}

Expand Down Expand Up @@ -937,6 +950,7 @@ impl CliOptions {
npmrc,
Arc::new(start_dir),
false,
None,
)
}

Expand Down
14 changes: 11 additions & 3 deletions cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1410,9 +1410,17 @@ impl ConfigData {
.unwrap_or_default(),
);

let ts_config = LspTsConfig::new(
member_dir.workspace.root_deno_json().map(|c| c.as_ref()),
);
// TODO(nayeemrmn): This is a hack to get member-specific compiler options.
let ts_config = if let Some(config_file) = member_dir
.maybe_deno_json()
.filter(|c| c.json.compiler_options.is_some())
{
LspTsConfig::new(Some(config_file))
} else {
LspTsConfig::new(
member_dir.workspace.root_deno_json().map(|c| c.as_ref()),
)
};

let deno_lint_config =
if ts_config.inner.0.get("jsx").and_then(|v| v.as_str()) == Some("react")
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 @@ -3681,6 +3681,7 @@ impl Inner {
.unwrap_or_else(create_default_npmrc),
workspace,
force_global_cache,
None,
)?;

let open_docs = self.documents.documents(DocumentsFilter::OpenDiagnosable);
Expand Down
210 changes: 205 additions & 5 deletions cli/tools/check.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use std::collections::BTreeMap;
use std::collections::BTreeSet;
use std::collections::HashSet;
use std::collections::VecDeque;
use std::sync::Arc;

use deno_ast::MediaType;
use deno_ast::ModuleSpecifier;
use deno_config::deno_json::get_ts_config_for_emit;
use deno_config::glob::PathOrPattern;
use deno_core::anyhow::anyhow;
use deno_core::error::AnyError;
use deno_graph::Module;
use deno_graph::ModuleGraph;
Expand All @@ -15,9 +20,15 @@ use once_cell::sync::Lazy;
use regex::Regex;

use crate::args::check_warn_tsconfig;
use crate::args::discover_npmrc_from_workspace;
use crate::args::CheckFlags;
use crate::args::CliLockfile;
use crate::args::CliOptions;
use crate::args::ConfigFlag;
use crate::args::FileFlags;
use crate::args::Flags;
use crate::args::LintFlags;
use crate::args::ScopeOptions;
use crate::args::TsConfig;
use crate::args::TsConfigType;
use crate::args::TsTypeLib;
Expand All @@ -40,6 +51,159 @@ pub async fn check(
flags: Arc<Flags>,
check_flags: CheckFlags,
) -> Result<(), AnyError> {
let is_discovered_config = match flags.config_flag {
ConfigFlag::Discover => true,
ConfigFlag::Path(_) => false,
ConfigFlag::Disabled => false,
};
if is_discovered_config {
let factory = CliFactory::from_flags(flags.clone());
let cli_options = factory.cli_options()?;
let (remote_files, files) = check_flags
.files
.iter()
.cloned()
.partition::<Vec<_>, _>(|f| {
f.starts_with("http://")
|| f.starts_with("https://")
|| f.starts_with("npm:")
|| f.starts_with("jsr:")
});
let cwd_prefix = format!(
"{}{}",
cli_options.initial_cwd().to_string_lossy(),
std::path::MAIN_SEPARATOR
);
// TODO(nayeemrmn): Using lint options for now. Add proper API to deno_config.
let mut by_workspace_directory = cli_options
.resolve_lint_options_for_members(&LintFlags {
files: FileFlags {
ignore: Default::default(),
include: files,
},
..Default::default()
})?
.into_iter()
.map(|(d, o)| {
let files = o
.files
.include
.iter()
.flat_map(|p| {
p.inner().iter().flat_map(|p| match p {
PathOrPattern::NegatedPath(_) => None,
PathOrPattern::Path(p) => Some(p.to_string_lossy().to_string()),
PathOrPattern::Pattern(p) => {
// TODO(nayeemrmn): Absolute globs don't work for specifier
// collection, we make them relative here for now.
let s = p.as_str();
Some(s.strip_prefix(&cwd_prefix).unwrap_or(&s).to_string())
}
PathOrPattern::RemoteUrl(_) => None,
})
})
.collect::<Vec<_>>();
(d.dir_url().clone(), (Arc::new(d), files))
})
.collect::<BTreeMap<_, _>>();
if !remote_files.is_empty() {
by_workspace_directory
.entry(cli_options.start_dir.dir_url().clone())
.or_insert((cli_options.start_dir.clone(), vec![]))
.1
.extend(remote_files);
}
let all_scopes = Arc::new(
by_workspace_directory
.iter()
.filter(|(_, (d, _))| d.has_deno_or_pkg_json())
.map(|(s, (_, _))| s.clone())
.collect::<BTreeSet<_>>(),
);
let dir_count = by_workspace_directory.len();
let mut check_errors = vec![];
let mut found_specifiers = false;
for (dir_url, (workspace_directory, files)) in by_workspace_directory {
let check_flags = CheckFlags {
files,
doc: check_flags.doc,
doc_only: check_flags.doc_only,
};
let (npmrc, _) =
discover_npmrc_from_workspace(&workspace_directory.workspace)?;
let lockfile =
CliLockfile::discover(&flags, &workspace_directory.workspace)?;
let scope_options = (dir_count > 1).then(|| ScopeOptions {
scope: workspace_directory
.has_deno_or_pkg_json()
.then(|| dir_url.clone()),
all_scopes: all_scopes.clone(),
});
let cli_options = CliOptions::new(
flags.clone(),
cli_options.initial_cwd().to_path_buf(),
lockfile.map(Arc::new),
npmrc,
workspace_directory,
false,
scope_options.map(Arc::new),
)?;
let factory = CliFactory::from_cli_options(Arc::new(cli_options));
let main_graph_container = factory.main_module_graph_container().await?;
let specifiers =
main_graph_container.collect_specifiers(&check_flags.files)?;
if specifiers.is_empty() {
continue;
} else {
found_specifiers = true;
}
let specifiers_for_typecheck = if check_flags.doc || check_flags.doc_only
{
let file_fetcher = factory.file_fetcher()?;
let root_permissions = factory.root_permissions_container()?;
let mut specifiers_for_typecheck = if check_flags.doc {
specifiers.clone()
} else {
vec![]
};
for s in specifiers {
let file = file_fetcher.fetch(&s, root_permissions).await?;
let snippet_files = extract::extract_snippet_files(file)?;
for snippet_file in snippet_files {
specifiers_for_typecheck.push(snippet_file.specifier.clone());
file_fetcher.insert_memory_files(snippet_file);
}
}

specifiers_for_typecheck
} else {
specifiers
};
if let Err(err) = main_graph_container
.check_specifiers(&specifiers_for_typecheck, None)
.await
{
check_errors.push(err);
}
}
if !found_specifiers {
log::warn!("{} No matching files found.", colors::yellow("Warning"));
}
if !check_errors.is_empty() {
// TODO(nayeemrmn): More integrated way of concatenating diagnostics from
// different checks.
return Err(anyhow!(
"{}",
check_errors
.into_iter()
.map(|e| e.to_string())
.collect::<Vec<_>>()
.join("\n\n"),
));
}
return Ok(());
}

let factory = CliFactory::from_flags(flags);

let main_graph_container = factory.main_module_graph_container().await?;
Expand Down Expand Up @@ -168,9 +332,22 @@ impl TypeChecker {
}

log::debug!("Type checking.");
let ts_config_result = self
// TODO(nayeemrmn): This is a hack to get member-specific compiler options.
let ts_config_result = if let Some(config_file) = self
.cli_options
.resolve_ts_config_for_emit(TsConfigType::Check { lib: options.lib })?;
.start_dir
.maybe_deno_json()
.filter(|c| c.json.compiler_options.is_some())
{
get_ts_config_for_emit(
TsConfigType::Check { lib: options.lib },
Some(config_file),
)?
} else {
self
.cli_options
.resolve_ts_config_for_emit(TsConfigType::Check { lib: options.lib })?
};
if options.log_ignored_options {
check_warn_tsconfig(&ts_config_result);
}
Expand Down Expand Up @@ -259,10 +436,33 @@ impl TypeChecker {

let mut diagnostics = response.diagnostics.filter(|d| {
if self.is_remote_diagnostic(d) {
type_check_mode == TypeCheckMode::All && d.include_when_remote()
} else {
true
return type_check_mode == TypeCheckMode::All
&& d.include_when_remote()
&& self
.cli_options
.scope_options
.as_ref()
.map(|o| o.scope.is_none())
.unwrap_or(true);
}
let Some(scope_options) = &self.cli_options.scope_options else {
return true;
};
let Some(specifier) = d
.file_name
.as_ref()
.and_then(|s| ModuleSpecifier::parse(s).ok())
else {
return true;
};
if specifier.scheme() != "file" {
return true;
}
let scope = scope_options
.all_scopes
.iter()
.rfind(|s| specifier.as_str().starts_with(s.as_str()));
scope == scope_options.scope.as_ref()
});

diagnostics.apply_fast_check_source_maps(&graph);
Expand Down
14 changes: 14 additions & 0 deletions tests/specs/check/check_workspace/__test__.jsonc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"tests": {
"discover": {
"args": "check --quiet main.ts member/mod.ts",
"output": "check_discover.out",
"exitCode": 1
},
"config_flag": {
"args": "check --quiet --config deno.json main.ts member/mod.ts",
"output": "check_config_flag.out",
"exitCode": 1
}
}
}
11 changes: 11 additions & 0 deletions tests/specs/check/check_workspace/check_config_flag.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
error: TS2304 [ERROR]: Cannot find name 'onmessage'.
onmessage;
~~~~~~~~~
at file:///home/nayeem/projects/deno/tests/specs/check/check_workspace/main.ts:8:1

TS2304 [ERROR]: Cannot find name 'onmessage'.
onmessage;
~~~~~~~~~
at file:///home/nayeem/projects/deno/tests/specs/check/check_workspace/member/mod.ts:5:1

Found 2 errors.
9 changes: 9 additions & 0 deletions tests/specs/check/check_workspace/check_discover.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
error: TS2304 [ERROR]: Cannot find name 'onmessage'.
onmessage;
~~~~~~~~~
at file:///[WILDCARD]/main.ts:8:1

TS2304 [ERROR]: Cannot find name 'localStorage'.
localStorage;
~~~~~~~~~~~~
at file:///[WILDCARD]/member/mod.ts:2:1
3 changes: 3 additions & 0 deletions tests/specs/check/check_workspace/deno.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"workspace": ["member"]
}
8 changes: 8 additions & 0 deletions tests/specs/check/check_workspace/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// We shouldn't get diagnostics from this import under this check scope.
import "./member/mod.ts";

// Only defined for window.
localStorage;

// Only defined for worker.
onmessage;
5 changes: 5 additions & 0 deletions tests/specs/check/check_workspace/member/deno.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"compilerOptions": {
"lib": ["deno.worker"]
}
}
Loading
Loading