diff options
author | Zheyu Zhang <zheyuzhang03@gmail.com> | 2021-11-04 23:12:12 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-11-04 11:12:12 -0400 |
commit | efe956b4fd51818e3a982b9d7c4111c408d07291 (patch) | |
tree | 9e3bf26c021758e962473468a7e954fc9cb63937 | |
parent | 318dcc33afd510c02984d4d3527c88bf4383bcf1 (diff) |
fix(lint): use recommended tag if there is no tags in config file or flags (#12644)
-rw-r--r-- | cli/flags.rs | 72 | ||||
-rw-r--r-- | cli/lsp/analysis.rs | 3 | ||||
-rw-r--r-- | cli/tests/integration/lint_tests.rs | 6 | ||||
-rw-r--r-- | cli/tests/testdata/lint/Deno.no_tags.jsonc | 17 | ||||
-rw-r--r-- | cli/tests/testdata/lint/with_config_without_tags.out | 18 | ||||
-rw-r--r-- | cli/tests/testdata/lsp/deno.lint.jsonc | 9 | ||||
-rw-r--r-- | cli/tests/testdata/lsp/types.tsconfig.json | 5 | ||||
-rw-r--r-- | cli/tools/lint.rs | 42 |
8 files changed, 111 insertions, 61 deletions
diff --git a/cli/flags.rs b/cli/flags.rs index 67c76244d..3edd2f0db 100644 --- a/cli/flags.rs +++ b/cli/flags.rs @@ -121,9 +121,9 @@ pub struct LintFlags { pub files: Vec<PathBuf>, pub ignore: Vec<PathBuf>, pub rules: bool, - pub rules_tags: Vec<String>, - pub rules_include: Vec<String>, - pub rules_exclude: Vec<String>, + pub maybe_rules_tags: Option<Vec<String>>, + pub maybe_rules_include: Option<Vec<String>>, + pub maybe_rules_exclude: Option<Vec<String>>, pub json: bool, } @@ -1976,25 +1976,25 @@ fn lint_parse(flags: &mut Flags, matches: &clap::ArgMatches) { None => vec![], }; let rules = matches.is_present("rules"); - let rules_tags = match matches.values_of("rules-tags") { - Some(f) => f.map(String::from).collect(), - None => vec![], - }; - let rules_include = match matches.values_of("rules-include") { - Some(f) => f.map(String::from).collect(), - None => vec![], - }; - let rules_exclude = match matches.values_of("rules-exclude") { - Some(f) => f.map(String::from).collect(), - None => vec![], - }; + let maybe_rules_tags = matches + .values_of("rules-tags") + .map(|f| f.map(String::from).collect()); + + let maybe_rules_include = matches + .values_of("rules-include") + .map(|f| f.map(String::from).collect()); + + let maybe_rules_exclude = matches + .values_of("rules-exclude") + .map(|f| f.map(String::from).collect()); + let json = matches.is_present("json"); flags.subcommand = DenoSubcommand::Lint(LintFlags { files, rules, - rules_tags, - rules_include, - rules_exclude, + maybe_rules_tags, + maybe_rules_include, + maybe_rules_exclude, ignore, json, }); @@ -2826,9 +2826,9 @@ mod tests { PathBuf::from("script_2.ts") ], rules: false, - rules_tags: vec![], - rules_include: vec![], - rules_exclude: vec![], + maybe_rules_tags: None, + maybe_rules_include: None, + maybe_rules_exclude: None, json: false, ignore: vec![], }), @@ -2844,9 +2844,9 @@ mod tests { subcommand: DenoSubcommand::Lint(LintFlags { files: vec![], rules: false, - rules_tags: vec![], - rules_include: vec![], - rules_exclude: vec![], + maybe_rules_tags: None, + maybe_rules_include: None, + maybe_rules_exclude: None, json: false, ignore: vec![ PathBuf::from("script_1.ts"), @@ -2864,9 +2864,9 @@ mod tests { subcommand: DenoSubcommand::Lint(LintFlags { files: vec![], rules: true, - rules_tags: vec![], - rules_include: vec![], - rules_exclude: vec![], + maybe_rules_tags: None, + maybe_rules_include: None, + maybe_rules_exclude: None, json: false, ignore: vec![], }), @@ -2887,9 +2887,9 @@ mod tests { subcommand: DenoSubcommand::Lint(LintFlags { files: vec![], rules: false, - rules_tags: svec![""], - rules_include: svec!["ban-untagged-todo", "no-undef"], - rules_exclude: svec!["no-const-assign"], + maybe_rules_tags: Some(svec![""]), + maybe_rules_include: Some(svec!["ban-untagged-todo", "no-undef"]), + maybe_rules_exclude: Some(svec!["no-const-assign"]), json: false, ignore: vec![], }), @@ -2904,9 +2904,9 @@ mod tests { subcommand: DenoSubcommand::Lint(LintFlags { files: vec![PathBuf::from("script_1.ts")], rules: false, - rules_tags: vec![], - rules_include: vec![], - rules_exclude: vec![], + maybe_rules_tags: None, + maybe_rules_include: None, + maybe_rules_exclude: None, json: true, ignore: vec![], }), @@ -2928,9 +2928,9 @@ mod tests { subcommand: DenoSubcommand::Lint(LintFlags { files: vec![PathBuf::from("script_1.ts")], rules: false, - rules_tags: vec![], - rules_include: vec![], - rules_exclude: vec![], + maybe_rules_tags: None, + maybe_rules_include: None, + maybe_rules_exclude: None, json: true, ignore: vec![], }), diff --git a/cli/lsp/analysis.rs b/cli/lsp/analysis.rs index 709131689..011ffa757 100644 --- a/cli/lsp/analysis.rs +++ b/cli/lsp/analysis.rs @@ -132,8 +132,7 @@ pub fn get_lint_references( parsed_source: &deno_ast::ParsedSource, maybe_lint_config: Option<&LintConfig>, ) -> Result<Vec<Reference>, AnyError> { - let lint_rules = - get_configured_rules(maybe_lint_config, vec![], vec![], vec![])?; + let lint_rules = get_configured_rules(maybe_lint_config, None, None, None)?; let linter = create_linter(parsed_source.media_type(), lint_rules); let lint_diagnostics = linter.lint_with_ast(parsed_source); diff --git a/cli/tests/integration/lint_tests.rs b/cli/tests/integration/lint_tests.rs index 4f4cf1984..3fee16aca 100644 --- a/cli/tests/integration/lint_tests.rs +++ b/cli/tests/integration/lint_tests.rs @@ -95,6 +95,12 @@ itest!(lint_with_config_and_flags { exit_code: 1, }); +itest!(lint_with_config_without_tags { + args: "lint --config lint/Deno.no_tags.jsonc lint/with_config/", + output: "lint/with_config_without_tags.out", + exit_code: 1, +}); + itest!(lint_with_malformed_config { args: "lint --config lint/Deno.malformed.jsonc", output: "lint/with_malformed_config.out", diff --git a/cli/tests/testdata/lint/Deno.no_tags.jsonc b/cli/tests/testdata/lint/Deno.no_tags.jsonc new file mode 100644 index 000000000..b86c0f8a8 --- /dev/null +++ b/cli/tests/testdata/lint/Deno.no_tags.jsonc @@ -0,0 +1,17 @@ +{ + "lint": { + "files": { + "include": [ + "lint/with_config/" + ], + "exclude": [ + "lint/with_config/b.ts" + ] + }, + "rules": { + "include": [ + "ban-untagged-todo" + ] + } + } +} diff --git a/cli/tests/testdata/lint/with_config_without_tags.out b/cli/tests/testdata/lint/with_config_without_tags.out new file mode 100644 index 000000000..ea4581af8 --- /dev/null +++ b/cli/tests/testdata/lint/with_config_without_tags.out @@ -0,0 +1,18 @@ +(ban-untagged-todo) TODO should be tagged with (@username) or (#issue) +// TODO: foo +^^^^^^^^^^^^ + at [WILDCARD]a.ts:1:0 + + hint: Add a user tag or issue reference to the TODO comment, e.g. TODO(@djones), TODO(djones), TODO(#123) + help: for further information visit https://lint.deno.land/#ban-untagged-todo + +(no-unused-vars) `add` is never used +function add(a: number, b: number): number { + ^^^ + at [WILDCARD]a.ts:2:9 + + hint: If this is intentional, prefix it with an underscore like `_add` + help: for further information visit https://lint.deno.land/#no-unused-vars + +Found 2 problems +Checked 1 file diff --git a/cli/tests/testdata/lsp/deno.lint.jsonc b/cli/tests/testdata/lsp/deno.lint.jsonc index 51db1b5c7..03308ab5f 100644 --- a/cli/tests/testdata/lsp/deno.lint.jsonc +++ b/cli/tests/testdata/lsp/deno.lint.jsonc @@ -1,8 +1,13 @@ { "lint": { "rules": { - "exclude": ["camelcase"], - "include": ["ban-untagged-todo"] + "exclude": [ + "camelcase" + ], + "include": [ + "ban-untagged-todo" + ], + "tags": [] } } } diff --git a/cli/tests/testdata/lsp/types.tsconfig.json b/cli/tests/testdata/lsp/types.tsconfig.json index ba7f3344d..50de6939c 100644 --- a/cli/tests/testdata/lsp/types.tsconfig.json +++ b/cli/tests/testdata/lsp/types.tsconfig.json @@ -3,5 +3,10 @@ "types": [ "./a.d.ts" ] + }, + "lint": { + "rules": { + "tags": [] + } } } diff --git a/cli/tools/lint.rs b/cli/tools/lint.rs index 706ecbbd8..8a5f2460b 100644 --- a/cli/tools/lint.rs +++ b/cli/tools/lint.rs @@ -51,9 +51,9 @@ pub async fn lint( watch: bool, ) -> Result<(), AnyError> { let LintFlags { - rules_tags, - rules_include, - rules_exclude, + maybe_rules_tags, + maybe_rules_include, + maybe_rules_exclude, files: args, ignore, json, @@ -102,9 +102,9 @@ pub async fn lint( // and `--rules-include` CLI flag, only the flag value is taken into account. let lint_rules = get_configured_rules( maybe_lint_config.as_ref(), - rules_tags, - rules_include, - rules_exclude, + maybe_rules_tags, + maybe_rules_include, + maybe_rules_exclude, )?; let resolver = |changed: Option<Vec<PathBuf>>| { @@ -488,20 +488,20 @@ fn sort_diagnostics(diagnostics: &mut Vec<LintDiagnostic>) { pub(crate) fn get_configured_rules( maybe_lint_config: Option<&LintConfig>, - rules_tags: Vec<String>, - rules_include: Vec<String>, - rules_exclude: Vec<String>, + maybe_rules_tags: Option<Vec<String>>, + maybe_rules_include: Option<Vec<String>>, + maybe_rules_exclude: Option<Vec<String>>, ) -> Result<Vec<Arc<dyn LintRule>>, AnyError> { if maybe_lint_config.is_none() - && rules_tags.is_empty() - && rules_include.is_empty() - && rules_exclude.is_empty() + && maybe_rules_tags.is_none() + && maybe_rules_include.is_none() + && maybe_rules_exclude.is_none() { return Ok(rules::get_recommended_rules()); } let (config_file_tags, config_file_include, config_file_exclude) = - if let Some(lint_config) = maybe_lint_config.as_ref() { + if let Some(lint_config) = maybe_lint_config { ( lint_config.rules.tags.clone(), lint_config.rules.include.clone(), @@ -511,26 +511,26 @@ pub(crate) fn get_configured_rules( (None, None, None) }; - let maybe_configured_include = if !rules_include.is_empty() { - Some(rules_include) + let maybe_configured_include = if maybe_rules_include.is_some() { + maybe_rules_include } else { config_file_include }; - let maybe_configured_exclude = if !rules_exclude.is_empty() { - Some(rules_exclude) + let maybe_configured_exclude = if maybe_rules_exclude.is_some() { + maybe_rules_exclude } else { config_file_exclude }; - let configured_tags = if !rules_tags.is_empty() { - rules_tags + let maybe_configured_tags = if maybe_rules_tags.is_some() { + maybe_rules_tags } else { - config_file_tags.unwrap_or_else(Vec::new) + config_file_tags }; let configured_rules = rules::get_filtered_rules( - Some(configured_tags), + maybe_configured_tags, maybe_configured_exclude, maybe_configured_include, ); |