fix(lint): use recommended tag if there is no tags in config file or flags (#12644)

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![],
         }),

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);
 

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",

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"
+      ]
+    }
+  }
+}

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

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": []
     }
   }
 }

cli/tests/testdata/lsp/types.tsconfig.json

@@ -3,5 +3,10 @@
     "types": [
       "./a.d.ts"
     ]
+  },
+  "lint": {
+    "rules": {
+      "tags": []
+    }
   }
 }

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,
   );