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

feat(lint): change behavior of --rules flag #27245

Merged
merged 5 commits into from
Jan 22, 2025
Merged
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
2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ deno_doc = { version = "=0.164.0", features = ["rust", "comrak"] }
deno_error.workspace = true
deno_graph = { version = "=0.87.0" }
deno_lib.workspace = true
deno_lint = { version = "=0.68.2", features = ["docs"] }
deno_lint = { version = "=0.68.2" }
deno_lockfile.workspace = true
deno_npm.workspace = true
deno_npm_cache.workspace = true
Expand Down
47 changes: 28 additions & 19 deletions cli/tools/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -458,27 +458,27 @@ fn collect_lint_files(
#[allow(clippy::print_stdout)]
pub fn print_rules_list(json: bool, maybe_rules_tags: Option<Vec<String>>) {
let rule_provider = LintRuleProvider::new(None, None);
let lint_rules = rule_provider
.resolve_lint_rules(
LintRulesConfig {
tags: maybe_rules_tags.clone(),
include: None,
exclude: None,
},
None,
)
.rules;
let all_rules = rule_provider.all_rules();
let configured_rules = rule_provider.resolve_lint_rules(
LintRulesConfig {
tags: maybe_rules_tags.clone(),
include: None,
exclude: None,
},
None,
);

if json {
let json_output = serde_json::json!({
"version": JSON_SCHEMA_VERSION,
"rules": lint_rules
"rules": all_rules
.iter()
.map(|rule| {
// TODO(bartlomieju): print if rule enabled
serde_json::json!({
"code": rule.code(),
"tags": rule.tags(),
"docs": rule.docs(),
"docs": rule.help_docs_url(),
})
})
.collect::<Vec<serde_json::Value>>(),
Expand All @@ -488,17 +488,26 @@ pub fn print_rules_list(json: bool, maybe_rules_tags: Option<Vec<String>>) {
// The rules should still be printed even if `--quiet` option is enabled,
// so use `println!` here instead of `info!`.
println!("Available rules:");
for rule in lint_rules.iter() {
print!(" - {}", colors::cyan(rule.code()));
if rule.tags().is_empty() {
println!();
for rule in all_rules.iter() {
// TODO(bartlomieju): this is O(n) search, fix before landing
let enabled = if configured_rules.rules.contains(rule) {
"✓"
} else {
println!(" [{}]", colors::gray(rule.tags().join(", ")))
}
" "
};
println!("- {} {}", rule.code(), colors::green(enabled),);
println!(
"{}",
colors::gray(format!(" help: {}", rule.help_docs_url()))
colors::gray(format!(" help: {}", rule.help_docs_url()))
);
if rule.tags().is_empty() {
println!(" {}", colors::gray("tags:"));
} else {
println!(
" {}",
colors::gray(format!("tags: {}", rule.tags().join(", ")))
);
}
println!();
}
}
Expand Down
46 changes: 22 additions & 24 deletions cli/tools/lint/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,6 @@ pub trait PackageLintRule: std::fmt::Debug + Send + Sync {
&[]
}

fn docs(&self) -> &'static str;

fn help_docs_url(&self) -> Cow<'static, str>;

fn lint_package(
Expand Down Expand Up @@ -64,6 +62,12 @@ enum CliLintRuleKind {
#[derive(Debug)]
pub struct CliLintRule(CliLintRuleKind);

impl PartialEq for CliLintRule {
fn eq(&self, other: &Self) -> bool {
self.code() == other.code()
}
}

impl CliLintRule {
pub fn code(&self) -> &'static str {
use CliLintRuleKind::*;
Expand All @@ -83,15 +87,6 @@ impl CliLintRule {
}
}

pub fn docs(&self) -> &'static str {
use CliLintRuleKind::*;
match &self.0 {
DenoLint(rule) => rule.docs(),
Extended(rule) => rule.docs(),
Package(rule) => rule.docs(),
}
}

pub fn help_docs_url(&self) -> Cow<'static, str> {
use CliLintRuleKind::*;
match &self.0 {
Expand Down Expand Up @@ -171,11 +166,7 @@ impl LintRuleProvider {
Ok(lint_rules)
}

pub fn resolve_lint_rules(
&self,
rules: LintRulesConfig,
maybe_config_file: Option<&ConfigFile>,
) -> ConfiguredRules {
pub fn all_rules(&self) -> Vec<CliLintRule> {
let deno_lint_rules = deno_lint::rules::get_all_rules();
let cli_lint_rules = vec![CliLintRule(CliLintRuleKind::Extended(
Box::new(no_sloppy_imports::NoSloppyImportsRule::new(
Expand All @@ -186,19 +177,26 @@ impl LintRuleProvider {
let cli_graph_rules = vec![CliLintRule(CliLintRuleKind::Package(
Box::new(no_slow_types::NoSlowTypesRule),
))];
let mut all_rule_names = HashSet::with_capacity(
deno_lint_rules.len() + cli_lint_rules.len() + cli_graph_rules.len(),
);
let all_rules = deno_lint_rules
deno_lint_rules
.into_iter()
.map(|rule| CliLintRule(CliLintRuleKind::DenoLint(rule)))
.chain(cli_lint_rules)
.chain(cli_graph_rules)
.inspect(|rule| {
all_rule_names.insert(rule.code());
});
.collect()
}

pub fn resolve_lint_rules(
&self,
rules: LintRulesConfig,
maybe_config_file: Option<&ConfigFile>,
) -> ConfiguredRules {
let all_rules = self.all_rules();
let mut all_rule_names = HashSet::with_capacity(all_rules.len());
for rule in &all_rules {
all_rule_names.insert(rule.code());
}
let rules = filtered_rules(
all_rules,
all_rules.into_iter(),
rules
.tags
.or_else(|| Some(get_default_tags(maybe_config_file))),
Expand Down
7 changes: 4 additions & 3 deletions cli/tools/lint/rules/no_sloppy_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,10 @@ impl LintRule for NoSloppyImportsRule {
CODE
}

fn docs(&self) -> &'static str {
include_str!("no_sloppy_imports.md")
}
// TODO(bartlomieju): this document needs to be exposed to `https://lint.deno.land`.
// fn docs(&self) -> &'static str {
// include_str!("no_sloppy_imports.md")
// }

fn tags(&self) -> &'static [&'static str] {
&["recommended"]
Expand Down
7 changes: 4 additions & 3 deletions cli/tools/lint/rules/no_slow_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ impl PackageLintRule for NoSlowTypesRule {
&["jsr"]
}

fn docs(&self) -> &'static str {
include_str!("no_slow_types.md")
}
// TODO(bartlomieju): these docs need to be hosted somewhere.
// fn docs(&self) -> &'static str {
// include_str!("no_slow_types.md")
// }

fn help_docs_url(&self) -> Cow<'static, str> {
Cow::Borrowed("https://jsr.io/docs/about-slow-types")
Expand Down
Loading