allow suppressing lint rule from command line

Summary:
This diff adds a `--suppression` flag to `starlark_bin` `--check` mode so we can suppress some lint rules with specified file pattern.

```
      --suppression <SUPPRESSION>   Specify lint rules to suppress. You may specify an optional glob pattern to suppress rules for files matching the pattern, in the format of `<glob>:<rule>[,<rule>]*`.
```

Reviewed By: ndmitchell

Differential Revision: D57078917

fbshipit-source-id: aedbbcfd278b94833635bbf3de1030c70cb0c7ce

starlark_bin/Cargo.toml

@@ -25,6 +25,7 @@ argfile = "0.1.0"
 clap = { version = "4.0.7", features = ["derive", "wrap_help"] }
 debugserver-types = "0.5.0"
 either = "1.8"
+globset = "0.4.13"
 itertools = "0.10"
 lsp-types = "0.94.1"
 serde = { version = "1.0", features = ["derive"] }

starlark_bin/bin/eval.rs

@@ -44,6 +44,8 @@ use starlark_lsp::server::LspEvalResult;
 use starlark_lsp::server::LspUrl;
 use starlark_lsp::server::StringLiteralResult;
 
+use crate::suppression::GlobLintSuppression;
+
 #[derive(Debug)]
 pub(crate) enum ContextMode {
     Check,
@@ -70,6 +72,7 @@ pub(crate) struct Context {
     pub(crate) globals: Globals,
     pub(crate) builtin_docs: HashMap<LspUrl, String>,
     pub(crate) builtin_symbols: HashMap<String, LspUrl>,
+    pub(crate) suppression_rules: Vec<GlobLintSuppression>,
 }
 
 /// The outcome of evaluating (checking, parsing or running) given starlark code.
@@ -101,6 +104,7 @@ impl Context {
         module: bool,
         dialect: Dialect,
         globals: Globals,
+        suppression_rules: Vec<GlobLintSuppression>,
     ) -> anyhow::Result<Self> {
         let prelude: Vec<_> = prelude
             .iter()
@@ -143,6 +147,7 @@ impl Context {
             globals,
             builtin_docs,
             builtin_symbols,
+            suppression_rules,
         })
     }
 
@@ -172,7 +177,7 @@ impl Context {
         let mut errors = Either::Left(iter::empty());
         let final_ast = match self.mode {
             ContextMode::Check => {
-                warnings = Either::Right(self.check(&ast));
+                warnings = Either::Right(self.check(file, &ast));
                 Some(ast)
             }
             ContextMode::Run => {
@@ -266,7 +271,13 @@ impl Context {
         )
     }
 
-    fn check(&self, module: &AstModule) -> impl Iterator<Item = EvalMessage> {
+    fn is_suppressed(&self, file: &str, issue: &str) -> bool {
+        self.suppression_rules
+            .iter()
+            .any(|rule| rule.is_suppressed(file, issue))
+    }
+
+    fn check(&self, file: &str, module: &AstModule) -> impl Iterator<Item = EvalMessage> {
         let globals = if self.prelude.is_empty() {
             None
         } else {
@@ -284,10 +295,9 @@ impl Context {
             Some(globals)
         };
 
-        module
-            .lint(globals.as_ref())
-            .into_iter()
-            .map(EvalMessage::from)
+        let mut lints = module.lint(globals.as_ref());
+        lints.retain(|issue| !self.is_suppressed(file, &issue.short_name));
+        lints.into_iter().map(EvalMessage::from)
     }
 }
 

starlark_bin/bin/main.rs

@@ -26,6 +26,8 @@ use std::path::PathBuf;
 use std::sync::Arc;
 
 use anyhow::Context as _;
+use clap::builder::StringValueParser;
+use clap::builder::TypedValueParser;
 use clap::Parser;
 use clap::ValueEnum;
 use dupe::Dupe;
@@ -44,13 +46,15 @@ use starlark::errors::EvalMessage;
 use starlark::errors::EvalSeverity;
 use starlark::read_line::ReadLine;
 use starlark::syntax::Dialect;
+use suppression::GlobLintSuppression;
 use walkdir::WalkDir;
 
 use crate::eval::ContextMode;
 
 mod bazel;
 mod dap;
 mod eval;
+mod suppression;
 
 #[derive(Debug, Parser)]
 #[command(name = "starlark", about = "Evaluate Starlark code", version)]
@@ -147,6 +151,15 @@ struct Args {
         help = "Run in Bazel mode (temporary, will be removed)"
     )]
     bazel: bool,
+
+    #[arg(
+        long = "suppression",
+        help = "Specify lint rules to suppress. You may specify an optional glob pattern to \
+suppress rules for files matching the pattern, in the format of `[<glob>:]<rule>[,<rule>]*`.",
+        requires = "check",
+        value_parser = StringValueParser::new().try_map(GlobLintSuppression::try_parse)
+    )]
+    suppression: Vec<GlobLintSuppression>,
 }
 
 #[derive(ValueEnum, Copy, Clone, Dupe, Debug, PartialEq, Eq)]
@@ -313,6 +326,7 @@ fn main() -> anyhow::Result<()> {
             is_interactive,
             dialect,
             globals,
+            args.suppression,
         )?;
 
         if args.lsp {

starlark_bin/bin/suppression.rs

@@ -0,0 +1,79 @@
+/*
+ * Copyright 2019 The Starlark in Rust Authors.
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+use std::collections::HashSet;
+
+use globset::Glob;
+use globset::GlobMatcher;
+
+#[derive(Debug, Clone)]
+pub struct GlobLintSuppression {
+    /// Glob pattern to match the suppression rule on.
+    pattern: GlobMatcher,
+
+    /// List of rules to be suppressed. This should be the name specified in
+    /// [`starlark::analysis::Lint::short_name`].
+    rules: HashSet<String>,
+}
+
+impl GlobLintSuppression {
+    pub fn try_parse(input: impl AsRef<str>) -> anyhow::Result<Self> {
+        let rule = input.as_ref();
+        let (pattern, rules) = if let Some((lhs, rhs)) = rule.split_once(':') {
+            (lhs, rhs)
+        } else {
+            ("*", rule)
+        };
+
+        let rules = rules.split(',').map(|s| s.trim().to_owned()).collect();
+
+        Ok(Self {
+            pattern: Glob::new(pattern)?.compile_matcher(),
+            rules,
+        })
+    }
+
+    pub fn is_suppressed(&self, filename: &str, rule: &str) -> bool {
+        if self.pattern.is_match(filename) {
+            return self.rules.contains(rule);
+        }
+
+        false
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_parsing() {
+        let suppression = GlobLintSuppression::try_parse("*.bzl:rule1,rule2").unwrap();
+
+        assert!(suppression.is_suppressed("foo/bar.bzl", "rule1"));
+        assert!(suppression.is_suppressed("foo/bar.bzl", "rule2"));
+        assert!(!suppression.is_suppressed("foo/baz.star", "rule2"));
+    }
+
+    #[test]
+    fn test_parsing_default() {
+        let suppression = GlobLintSuppression::try_parse("rule1").unwrap();
+
+        assert!(suppression.is_suppressed("foo/bar.bzl", "rule1"));
+        assert!(!suppression.is_suppressed("foo/bar.bzl", "rule2"));
+    }
+}