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): support sloppy imports with "compilerOptions.rootDirs" #27973

Merged
merged 9 commits into from
Feb 5, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 3 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ edition = "2021"
license = "MIT"
repository = "https://github.com/denoland/deno"

# TODO(nayeemrmn): Land https://github.com/denoland/deno_config/pull/156!
[patch.crates-io]
deno_config = { git = "https://github.com/denoland/deno_config.git", rev = "1f6fb4e0d8ece73eeada791f2911aaa68088a076" }

nayeemrmn marked this conversation as resolved.
Show resolved Hide resolved
[workspace.dependencies]
deno_ast = { version = "=0.44.0", features = ["transpiling"] }
deno_core = { version = "0.333.0" }
Expand Down
15 changes: 3 additions & 12 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ use crate::resolver::CliDenoResolver;
use crate::resolver::CliNpmGraphResolver;
use crate::resolver::CliNpmReqResolver;
use crate::resolver::CliResolver;
use crate::resolver::CliSloppyImportsResolver;
use crate::resolver::FoundPackageJsonDepFlag;
use crate::standalone::binary::DenoCompileBinaryWriter;
use crate::sys::CliSys;
Expand Down Expand Up @@ -646,7 +645,6 @@ impl CliFactory {
ResolverFactoryOptions {
conditions_from_resolution_mode: Default::default(),
node_resolution_cache: Some(Arc::new(NodeResolutionThreadLocalCache)),
no_sloppy_imports_cache: false,
npm_system_info: self.flags.subcommand.npm_system_info(),
specified_import_map: Some(Box::new(CliSpecifiedImportMapProvider {
cli_options: self.cli_options()?.clone(),
Expand All @@ -672,12 +670,6 @@ impl CliFactory {
})
}

pub fn sloppy_imports_resolver(
&self,
) -> Result<Option<&Arc<CliSloppyImportsResolver>>, AnyError> {
self.resolver_factory()?.sloppy_imports_resolver()
}

pub fn workspace(&self) -> Result<&Arc<Workspace>, AnyError> {
Ok(&self.workspace_directory()?.workspace)
}
Expand Down Expand Up @@ -790,10 +782,9 @@ impl CliFactory {
}

pub async fn lint_rule_provider(&self) -> Result<LintRuleProvider, AnyError> {
Ok(LintRuleProvider::new(
self.sloppy_imports_resolver()?.cloned(),
Some(self.workspace_resolver().await?.clone()),
))
Ok(LintRuleProvider::new(Some(
self.workspace_resolver().await?.clone(),
)))
}

pub async fn node_resolver(&self) -> Result<&Arc<CliNodeResolver>, AnyError> {
Expand Down
13 changes: 7 additions & 6 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use deno_config::deno_json;
use deno_config::deno_json::CompilerOptionTypesDeserializeError;
use deno_config::deno_json::JsxImportSourceConfig;
use deno_config::deno_json::NodeModulesDirMode;
use deno_config::workspace::sloppy_imports_resolve;
use deno_config::workspace::JsrPackageConfig;
use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
Expand All @@ -34,8 +35,6 @@ use deno_graph::SpecifierError;
use deno_graph::WorkspaceFastCheckOption;
use deno_path_util::url_to_file_path;
use deno_resolver::npm::DenoInNpmPackageChecker;
use deno_resolver::sloppy_imports::SloppyImportsCachedFs;
use deno_resolver::sloppy_imports::SloppyImportsResolutionKind;
use deno_runtime::deno_node;
use deno_runtime::deno_permissions::PermissionsContainer;
use deno_semver::jsr::JsrDepPackageReq;
Expand All @@ -62,7 +61,6 @@ use crate::npm::CliNpmResolver;
use crate::resolver::CliCjsTracker;
use crate::resolver::CliNpmGraphResolver;
use crate::resolver::CliResolver;
use crate::resolver::CliSloppyImportsResolver;
use crate::sys::CliSys;
use crate::tools::check;
use crate::tools::check::CheckError;
Expand Down Expand Up @@ -949,11 +947,14 @@ pub fn maybe_additional_sloppy_imports_message(
sys: &CliSys,
specifier: &ModuleSpecifier,
) -> Option<String> {
let (resolved, sloppy_reason) = sloppy_imports_resolve(
specifier,
deno_config::workspace::ResolutionKind::Execution,
sys.clone(),
)?;
Some(format!(
"{} {}",
CliSloppyImportsResolver::new(SloppyImportsCachedFs::new(sys.clone()))
.resolve(specifier, SloppyImportsResolutionKind::Execution)?
.as_suggestion_message(),
sloppy_reason.suggestion_message_for_specifier(&resolved),
RUN_WITH_SLOPPY_IMPORTS_MSG
))
}
Expand Down
3 changes: 1 addition & 2 deletions cli/lsp/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1349,8 +1349,7 @@ impl CodeActionCollection {
deno_config::workspace::ResolutionKind::Execution,
) {
let specifier = match resolution {
MappedResolution::Normal { specifier, .. }
| MappedResolution::ImportMap { specifier, .. } => specifier,
MappedResolution::Normal { specifier, .. } => specifier,
_ => {
return None;
}
Expand Down
44 changes: 21 additions & 23 deletions cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ use deno_config::deno_json::TsConfigWithIgnoredOptions;
use deno_config::glob::FilePatterns;
use deno_config::glob::PathOrPatternSet;
use deno_config::workspace::CreateResolverOptions;
use deno_config::workspace::FsCacheOptions;
use deno_config::workspace::PackageJsonDepResolution;
use deno_config::workspace::SloppyImportsOptions;
use deno_config::workspace::SpecifiedImportMap;
use deno_config::workspace::VendorEnablement;
use deno_config::workspace::Workspace;
Expand All @@ -49,7 +51,6 @@ use deno_npm::npm_rc::ResolvedNpmRc;
use deno_package_json::PackageJsonCache;
use deno_path_util::url_to_file_path;
use deno_resolver::npmrc::discover_npmrc_from_workspace;
use deno_resolver::sloppy_imports::SloppyImportsCachedFs;
use deno_runtime::deno_node::PackageJson;
use indexmap::IndexSet;
use lsp_types::ClientCapabilities;
Expand All @@ -65,7 +66,6 @@ use crate::args::LintFlags;
use crate::args::LintOptions;
use crate::file_fetcher::CliFileFetcher;
use crate::lsp::logging::lsp_warn;
use crate::resolver::CliSloppyImportsResolver;
use crate::sys::CliSys;
use crate::tools::lint::CliLinter;
use crate::tools::lint::CliLinterOptions;
Expand Down Expand Up @@ -1206,7 +1206,6 @@ pub struct ConfigData {
pub lockfile: Option<Arc<CliLockfile>>,
pub npmrc: Option<Arc<ResolvedNpmRc>>,
pub resolver: Arc<WorkspaceResolver<CliSys>>,
pub sloppy_imports_resolver: Option<Arc<CliSloppyImportsResolver>>,
pub import_map_from_settings: Option<ModuleSpecifier>,
pub unstable: BTreeSet<String>,
watched_files: HashMap<ModuleSpecifier, ConfigWatchedFileType>,
Expand Down Expand Up @@ -1569,13 +1568,29 @@ impl ConfigData {
None
}
};
let unstable = member_dir
.workspace
.unstable_features()
.iter()
.chain(settings.unstable.as_deref())
.cloned()
.collect::<BTreeSet<_>>();
let unstable_sloppy_imports = std::env::var("DENO_UNSTABLE_SLOPPY_IMPORTS")
.is_ok()
|| unstable.contains("sloppy-imports");
let resolver = member_dir
.workspace
.create_resolver(
CliSys::default(),
CreateResolverOptions {
pkg_json_dep_resolution,
specified_import_map,
sloppy_imports_options: if unstable_sloppy_imports {
SloppyImportsOptions::Enabled
} else {
SloppyImportsOptions::Disabled
},
fs_cache_options: FsCacheOptions::Disabled,
},
)
.inspect_err(|err| {
Expand All @@ -1595,6 +1610,8 @@ impl ConfigData {
pkg_json_dep_resolution,
Default::default(),
Default::default(),
Default::default(),
Default::default(),
CliSys::default(),
)
});
Expand All @@ -1609,26 +1626,8 @@ impl ConfigData {
.join("\n")
);
}
let unstable = member_dir
.workspace
.unstable_features()
.iter()
.chain(settings.unstable.as_deref())
.cloned()
.collect::<BTreeSet<_>>();
let unstable_sloppy_imports = std::env::var("DENO_UNSTABLE_SLOPPY_IMPORTS")
.is_ok()
|| unstable.contains("sloppy-imports");
let sloppy_imports_resolver = unstable_sloppy_imports.then(|| {
Arc::new(CliSloppyImportsResolver::new(
SloppyImportsCachedFs::new_without_stat_cache(CliSys::default()),
))
});
let resolver = Arc::new(resolver);
let lint_rule_provider = LintRuleProvider::new(
sloppy_imports_resolver.clone(),
Some(resolver.clone()),
);
let lint_rule_provider = LintRuleProvider::new(Some(resolver.clone()));
let linter = Arc::new(CliLinter::new(CliLinterOptions {
configured_rules: lint_rule_provider.resolve_lint_rules(
LintOptions::resolve((*lint_config).clone(), &LintFlags::default())
Expand All @@ -1644,7 +1643,6 @@ impl ConfigData {
canonicalized_scope,
member_dir,
resolver,
sloppy_imports_resolver,
fmt_config,
lint_config,
test_config,
Expand Down
25 changes: 10 additions & 15 deletions cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::thread;

use deno_ast::MediaType;
use deno_config::deno_json::LintConfig;
use deno_config::workspace::sloppy_imports_resolve;
use deno_core::anyhow::anyhow;
use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
Expand All @@ -27,9 +28,6 @@ use deno_graph::Resolution;
use deno_graph::ResolutionError;
use deno_graph::SpecifierError;
use deno_lint::linter::LintConfig as DenoLintConfig;
use deno_resolver::sloppy_imports::SloppyImportsCachedFs;
use deno_resolver::sloppy_imports::SloppyImportsResolution;
use deno_resolver::sloppy_imports::SloppyImportsResolutionKind;
use deno_runtime::deno_node;
use deno_runtime::tokio_util::create_basic_runtime;
use deno_semver::jsr::JsrPackageReqReference;
Expand Down Expand Up @@ -64,7 +62,6 @@ use crate::graph_util;
use crate::graph_util::enhanced_resolution_error_message;
use crate::lsp::logging::lsp_warn;
use crate::lsp::lsp_custom::DiagnosticBatchNotificationParams;
use crate::resolver::CliSloppyImportsResolver;
use crate::sys::CliSys;
use crate::tools::lint::CliLinter;
use crate::tools::lint::CliLinterOptions;
Expand Down Expand Up @@ -1013,7 +1010,7 @@ fn generate_lint_diagnostics(
Arc::new(LintConfig::new_with_base(PathBuf::from("/"))),
Arc::new(CliLinter::new(CliLinterOptions {
configured_rules: {
let lint_rule_provider = LintRuleProvider::new(None, None);
let lint_rule_provider = LintRuleProvider::new(None);
lint_rule_provider.resolve_lint_rules(Default::default(), None)
},
fix: false,
Expand Down Expand Up @@ -1439,14 +1436,14 @@ impl DenoDiagnostic {
pub fn to_lsp_diagnostic(&self, range: &lsp::Range) -> lsp::Diagnostic {
fn no_local_message(
specifier: &ModuleSpecifier,
maybe_sloppy_resolution: Option<&SloppyImportsResolution>,
suggestion_message: Option<String>,
) -> String {
let mut message = format!(
"Unable to load a local module: {}\n",
to_percent_decoded_str(specifier.as_ref())
);
if let Some(res) = maybe_sloppy_resolution {
message.push_str(&res.as_suggestion_message());
if let Some(suggestion_message) = suggestion_message {
message.push_str(&suggestion_message);
message.push('.');
} else {
message.push_str("Please check the file path.");
Expand All @@ -1463,17 +1460,15 @@ impl DenoDiagnostic {
Self::NotInstalledJsr(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("JSR package \"{pkg_req}\" is not installed or doesn't exist."), Some(json!({ "specifier": specifier }))),
Self::NotInstalledNpm(pkg_req, specifier) => (lsp::DiagnosticSeverity::ERROR, format!("npm package \"{pkg_req}\" is not installed or doesn't exist."), Some(json!({ "specifier": specifier }))),
Self::NoLocal(specifier) => {
let maybe_sloppy_resolution = CliSloppyImportsResolver::new(
SloppyImportsCachedFs::new(CliSys::default())
).resolve(specifier, SloppyImportsResolutionKind::Execution);
let data = maybe_sloppy_resolution.as_ref().map(|res| {
let sloppy_resolution = sloppy_imports_resolve(specifier, deno_config::workspace::ResolutionKind::Execution, CliSys::default());
let data = sloppy_resolution.as_ref().map(|(resolved, sloppy_reason)| {
json!({
"specifier": specifier,
"to": res.as_specifier(),
"message": res.as_quick_fix_message(),
"to": resolved,
"message": sloppy_reason.quick_fix_message_for_specifier(resolved),
})
});
(lsp::DiagnosticSeverity::ERROR, no_local_message(specifier, maybe_sloppy_resolution.as_ref()), data)
(lsp::DiagnosticSeverity::ERROR, no_local_message(specifier, sloppy_resolution.as_ref().map(|(resolved, sloppy_reason)| sloppy_reason.suggestion_message_for_specifier(resolved))), data)
},
Self::Redirect { from, to} => (lsp::DiagnosticSeverity::INFORMATION, format!("The import of \"{from}\" was redirected to \"{to}\"."), Some(json!({ "specifier": from, "redirect": to }))),
Self::ResolutionError(err) => {
Expand Down
5 changes: 2 additions & 3 deletions cli/lsp/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -844,9 +844,6 @@ impl<'a> ResolverFactory<'a> {
}
_ => None,
},
sloppy_imports_resolver: self
.config_data
.and_then(|d| d.sloppy_imports_resolver.clone()),
workspace_resolver: self
.config_data
.map(|d| d.resolver.clone())
Expand All @@ -860,6 +857,8 @@ impl<'a> ResolverFactory<'a> {
PackageJsonDepResolution::Disabled,
Default::default(),
Default::default(),
Default::default(),
Default::default(),
self.sys.clone(),
))
}),
Expand Down
6 changes: 0 additions & 6 deletions cli/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ use deno_graph::NpmLoadError;
use deno_graph::NpmResolvePkgReqsResult;
use deno_npm::resolution::NpmResolutionError;
use deno_resolver::npm::DenoInNpmPackageChecker;
use deno_resolver::sloppy_imports::SloppyImportsCachedFs;
use deno_resolver::sloppy_imports::SloppyImportsResolver;
use deno_runtime::colors;
use deno_runtime::deno_node::is_builtin_node_module;
use deno_semver::package::PackageReq;
Expand All @@ -35,14 +33,10 @@ pub type CliCjsTracker =
deno_resolver::cjs::CjsTracker<DenoInNpmPackageChecker, CliSys>;
pub type CliIsCjsResolver =
deno_resolver::cjs::IsCjsResolver<DenoInNpmPackageChecker, CliSys>;
pub type CliSloppyImportsCachedFs = SloppyImportsCachedFs<CliSys>;
pub type CliSloppyImportsResolver =
SloppyImportsResolver<CliSloppyImportsCachedFs>;
pub type CliDenoResolver = deno_resolver::DenoResolver<
DenoInNpmPackageChecker,
DenoIsBuiltInNodeModuleChecker,
CliNpmResolver,
CliSloppyImportsCachedFs,
CliSys,
>;
pub type CliNpmReqResolver = deno_resolver::npm::NpmReqResolver<
Expand Down
Loading
Loading