Skip to content

Commit

Permalink
fix(unstable): move sloppy-import warnings to lint rule (#24710)
Browse files Browse the repository at this point in the history
Adds a new `no-sloppy-imports` lint rule and cleans up the lint code.

Closes #22844
Closes denoland/deno_lint#1293
  • Loading branch information
dsherret committed Jul 26, 2024
1 parent f726fe8 commit 2a026be
Show file tree
Hide file tree
Showing 63 changed files with 1,601 additions and 955 deletions.
4 changes: 2 additions & 2 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"]
deno_doc = { version = "0.144.0", features = ["html", "syntect"] }
deno_emit = "=0.43.1"
deno_graph = { version = "=0.80.1", features = ["tokio_executor"] }
deno_lint = { version = "=0.60.1", features = ["docs"] }
deno_lint = { version = "=0.61.0", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm = "=0.21.4"
deno_package_json.workspace = true
Expand Down
32 changes: 27 additions & 5 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ use crate::resolver::SloppyImportsResolver;
use crate::standalone::DenoCompileBinaryWriter;
use crate::tools::check::TypeChecker;
use crate::tools::coverage::CoverageCollector;
use crate::tools::lint::LintRuleProvider;
use crate::tools::run::hmr::HmrRunner;
use crate::util::file_watcher::WatcherCommunicator;
use crate::util::fs::canonicalize_path_maybe_not_exists;
Expand Down Expand Up @@ -179,6 +180,7 @@ struct CliFactoryServices {
node_code_translator: Deferred<Arc<CliNodeCodeTranslator>>,
node_resolver: Deferred<Arc<NodeResolver>>,
npm_resolver: Deferred<Arc<dyn CliNpmResolver>>,
sloppy_imports_resolver: Deferred<Option<Arc<SloppyImportsResolver>>>,
text_only_progress_bar: Deferred<ProgressBar>,
type_checker: Deferred<Arc<TypeChecker>>,
cjs_resolutions: Deferred<Arc<CjsResolutionStore>>,
Expand Down Expand Up @@ -397,6 +399,23 @@ impl CliFactory {
.await
}

pub fn sloppy_imports_resolver(
&self,
) -> Result<Option<&Arc<SloppyImportsResolver>>, AnyError> {
self
.services
.sloppy_imports_resolver
.get_or_try_init(|| {
Ok(
self
.cli_options()?
.unstable_sloppy_imports()
.then(|| Arc::new(SloppyImportsResolver::new(self.fs().clone()))),
)
})
.map(|maybe| maybe.as_ref())
}

pub async fn workspace_resolver(
&self,
) -> Result<&Arc<WorkspaceResolver>, AnyError> {
Expand Down Expand Up @@ -440,11 +459,7 @@ impl CliFactory {
async {
let cli_options = self.cli_options()?;
Ok(Arc::new(CliGraphResolver::new(CliGraphResolverOptions {
sloppy_imports_resolver: if cli_options.unstable_sloppy_imports() {
Some(SloppyImportsResolver::new(self.fs().clone()))
} else {
None
},
sloppy_imports_resolver: self.sloppy_imports_resolver()?.cloned(),
node_resolver: Some(self.cli_node_resolver().await?.clone()),
npm_resolver: if cli_options.no_npm() {
None
Expand Down Expand Up @@ -524,6 +539,13 @@ 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()),
))
}

pub async fn node_resolver(&self) -> Result<&Arc<NodeResolver>, AnyError> {
self
.services
Expand Down
4 changes: 2 additions & 2 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -750,8 +750,8 @@ fn enhanced_sloppy_imports_error_message(
ModuleError::LoadingErr(specifier, _, ModuleLoadError::Loader(_)) // ex. "Is a directory" error
| ModuleError::Missing(specifier, _) => {
let additional_message = SloppyImportsResolver::new(fs.clone())
.resolve(specifier, ResolutionMode::Execution)
.as_suggestion_message()?;
.resolve(specifier, ResolutionMode::Execution)?
.as_suggestion_message();
Some(format!(
"{} {} or run with --unstable-sloppy-imports",
error,
Expand Down
66 changes: 34 additions & 32 deletions cli/lsp/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use super::resolver::LspResolver;
use super::tsc;

use crate::args::jsr_url;
use crate::tools::lint::create_linter;
use deno_lint::linter::LintConfig;
use crate::tools::lint::CliLinter;
use deno_lint::diagnostic::LintDiagnosticRange;
use deno_runtime::fs_util::specifier_to_file_path;

use deno_ast::SourceRange;
Expand All @@ -23,8 +23,6 @@ use deno_core::serde::Serialize;
use deno_core::serde_json;
use deno_core::serde_json::json;
use deno_core::ModuleSpecifier;
use deno_lint::diagnostic::LintDiagnostic;
use deno_lint::rules::LintRule;
use deno_runtime::deno_node::NpmResolver;
use deno_runtime::deno_node::PathClean;
use deno_semver::jsr::JsrPackageNvReference;
Expand Down Expand Up @@ -149,8 +147,10 @@ impl Reference {
}
}

fn as_lsp_range_from_diagnostic(diagnostic: &LintDiagnostic) -> Range {
as_lsp_range(diagnostic.range, &diagnostic.text_info)
fn as_lsp_range_from_lint_diagnostic(
diagnostic_range: &LintDiagnosticRange,
) -> Range {
as_lsp_range(diagnostic_range.range, &diagnostic_range.text_info)
}

fn as_lsp_range(
Expand All @@ -173,37 +173,39 @@ fn as_lsp_range(

pub fn get_lint_references(
parsed_source: &deno_ast::ParsedSource,
lint_rules: Vec<&'static dyn LintRule>,
lint_config: LintConfig,
linter: &CliLinter,
) -> Result<Vec<Reference>, AnyError> {
let linter = create_linter(lint_rules);
let lint_diagnostics = linter.lint_with_ast(parsed_source, lint_config);
let lint_diagnostics = linter.lint_with_ast(parsed_source);

Ok(
lint_diagnostics
.into_iter()
.map(|d| Reference {
range: as_lsp_range_from_diagnostic(&d),
category: Category::Lint {
message: d.message,
code: d.code,
hint: d.hint,
quick_fixes: d
.fixes
.into_iter()
.map(|f| DataQuickFix {
description: f.description.to_string(),
changes: f
.changes
.into_iter()
.map(|change| DataQuickFixChange {
range: as_lsp_range(change.range, &d.text_info),
new_text: change.new_text.to_string(),
})
.collect(),
})
.collect(),
},
.filter_map(|d| {
let range = d.range.as_ref()?;
Some(Reference {
range: as_lsp_range_from_lint_diagnostic(range),
category: Category::Lint {
message: d.details.message,
code: d.details.code.to_string(),
hint: d.details.hint,
quick_fixes: d
.details
.fixes
.into_iter()
.map(|f| DataQuickFix {
description: f.description.to_string(),
changes: f
.changes
.into_iter()
.map(|change| DataQuickFixChange {
range: as_lsp_range(change.range, &range.text_info),
new_text: change.new_text.to_string(),
})
.collect(),
})
.collect(),
},
})
})
.collect(),
)
Expand Down
45 changes: 34 additions & 11 deletions cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,16 @@ use crate::args::discover_npmrc_from_workspace;
use crate::args::has_flag_env_var;
use crate::args::CliLockfile;
use crate::args::ConfigFile;
use crate::args::LintFlags;
use crate::args::LintOptions;
use crate::args::DENO_FUTURE;
use crate::cache::FastInsecureHasher;
use crate::file_fetcher::FileFetcher;
use crate::lsp::logging::lsp_warn;
use crate::tools::lint::get_configured_rules;
use crate::tools::lint::ConfiguredRules;
use crate::resolver::SloppyImportsResolver;
use crate::tools::lint::CliLinter;
use crate::tools::lint::CliLinterOptions;
use crate::tools::lint::LintRuleProvider;
use crate::util::fs::canonicalize_path_maybe_not_exists;

pub const SETTINGS_SECTION: &str = "deno";
Expand Down Expand Up @@ -1116,15 +1120,15 @@ pub struct ConfigData {
pub lint_config: Arc<LintConfig>,
pub test_config: Arc<TestConfig>,
pub exclude_files: Arc<PathOrPatternSet>,
pub deno_lint_config: DenoLintConfig,
pub lint_rules: Arc<ConfiguredRules>,
pub linter: Arc<CliLinter>,
pub ts_config: Arc<LspTsConfig>,
pub byonm: bool,
pub node_modules_dir: Option<PathBuf>,
pub vendor_dir: Option<PathBuf>,
pub lockfile: Option<Arc<CliLockfile>>,
pub npmrc: Option<Arc<ResolvedNpmRc>>,
pub resolver: Arc<WorkspaceResolver>,
pub sloppy_imports_resolver: Option<Arc<SloppyImportsResolver>>,
pub import_map_from_settings: Option<ModuleSpecifier>,
watched_files: HashMap<ModuleSpecifier, ConfigWatchedFileType>,
}
Expand Down Expand Up @@ -1310,10 +1314,7 @@ impl ConfigData {
LintConfig::new_with_base(default_file_pattern_base.clone())
}),
);
let lint_rules = Arc::new(get_configured_rules(
lint_config.options.rules.clone(),
member_dir.maybe_deno_json().map(|c| c.as_ref()),
));

let test_config = Arc::new(
member_dir
.to_test_config(FilePatterns::new_with_base(member_dir.dir_path()))
Expand Down Expand Up @@ -1532,16 +1533,38 @@ impl ConfigData {
.join("\n")
);
}
let unstable_sloppy_imports = std::env::var("DENO_UNSTABLE_SLOPPY_IMPORTS")
.is_ok()
|| member_dir.workspace.has_unstable("sloppy-imports");
let sloppy_imports_resolver = unstable_sloppy_imports.then(|| {
Arc::new(SloppyImportsResolver::new_without_stat_cache(Arc::new(
deno_runtime::deno_fs::RealFs,
)))
});
let resolver = Arc::new(resolver);
let lint_rule_provider = LintRuleProvider::new(
sloppy_imports_resolver.clone(),
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())
.rules,
member_dir.maybe_deno_json().map(|c| c.as_ref()),
),
fix: false,
deno_lint_config,
}));

ConfigData {
scope,
member_dir,
resolver: Arc::new(resolver),
resolver,
sloppy_imports_resolver,
fmt_config,
lint_config,
test_config,
deno_lint_config,
lint_rules,
linter,
exclude_files,
ts_config: Arc::new(ts_config),
byonm,
Expand Down
Loading

0 comments on commit 2a026be

Please sign in to comment.