Make lint store its severity

Summary: Rather than having is_serious with unclear semantics, just store a severity level directly.

Reviewed By: stepancheg

Differential Revision: D47150377

fbshipit-source-id: 156fddb540d56b129a74db51190aa539fed572c4

Author: ndmitchell

starlark/src/analysis/dubious.rs

@@ -21,6 +21,7 @@ use thiserror::Error;
 
 use crate::analysis::types::LintT;
 use crate::analysis::types::LintWarning;
+use crate::analysis::EvalSeverity;
 use crate::codemap::CodeMap;
 use crate::codemap::FileSpan;
 use crate::codemap::Span;
@@ -43,8 +44,8 @@ pub(crate) enum Dubious {
 }
 
 impl LintWarning for Dubious {
-    fn is_serious(&self) -> bool {
-        true
+    fn severity(&self) -> EvalSeverity {
+        EvalSeverity::Warning
     }
 
     fn short_name(&self) -> &'static str {

starlark/src/analysis/flow.rs

@@ -19,6 +19,7 @@ use thiserror::Error;
 
 use crate::analysis::types::LintT;
 use crate::analysis::types::LintWarning;
+use crate::analysis::EvalSeverity;
 use crate::codemap::CodeMap;
 use crate::codemap::ResolvedFileSpan;
 use crate::codemap::Span;
@@ -51,11 +52,11 @@ pub(crate) enum FlowIssue {
 }
 
 impl LintWarning for FlowIssue {
-    fn is_serious(&self) -> bool {
+    fn severity(&self) -> EvalSeverity {
         match self {
             // Sometimes people add these to make flow clearer
-            FlowIssue::RedundantContinue | FlowIssue::RedundantReturn => false,
-            _ => true,
+            FlowIssue::RedundantContinue | FlowIssue::RedundantReturn => EvalSeverity::Disabled,
+            _ => EvalSeverity::Warning,
         }
     }
 

starlark/src/analysis/incompatible.rs

@@ -24,6 +24,7 @@ use thiserror::Error;
 
 use crate::analysis::types::LintT;
 use crate::analysis::types::LintWarning;
+use crate::analysis::EvalSeverity;
 use crate::codemap::CodeMap;
 use crate::codemap::FileSpan;
 use crate::codemap::Span;
@@ -46,8 +47,8 @@ pub(crate) enum Incompatibility {
 }
 
 impl LintWarning for Incompatibility {
-    fn is_serious(&self) -> bool {
-        true
+    fn severity(&self) -> EvalSeverity {
+        EvalSeverity::Warning
     }
 
     fn short_name(&self) -> &'static str {

starlark/src/analysis/names.rs

@@ -34,6 +34,7 @@ use thiserror::Error;
 
 use crate::analysis::types::LintT;
 use crate::analysis::types::LintWarning;
+use crate::analysis::EvalSeverity;
 use crate::codemap::CodeMap;
 use crate::codemap::Span;
 use crate::codemap::Spanned;
@@ -66,10 +67,10 @@ pub(crate) enum NameWarning {
 }
 
 impl LintWarning for NameWarning {
-    fn is_serious(&self) -> bool {
+    fn severity(&self) -> EvalSeverity {
         match self {
-            Self::UsingUnassigned(..) => true,
-            _ => false,
+            Self::UsingUnassigned(..) => EvalSeverity::Warning,
+            _ => EvalSeverity::Disabled,
         }
     }
 

starlark/src/analysis/performance.rs

@@ -19,6 +19,7 @@ use thiserror::Error;
 
 use crate::analysis::types::LintT;
 use crate::analysis::types::LintWarning;
+use crate::analysis::EvalSeverity;
 use crate::codemap::CodeMap;
 use crate::syntax::ast::Argument;
 use crate::syntax::ast::AstExpr;
@@ -40,8 +41,8 @@ pub(crate) enum Performance {
 }
 
 impl LintWarning for Performance {
-    fn is_serious(&self) -> bool {
-        true
+    fn severity(&self) -> EvalSeverity {
+        EvalSeverity::Warning
     }
 
     fn short_name(&self) -> &'static str {

starlark/src/analysis/types.rs

@@ -32,7 +32,7 @@ use crate::codemap::Span;
 use crate::errors::Diagnostic;
 
 pub(crate) trait LintWarning: Display {
-    fn is_serious(&self) -> bool;
+    fn severity(&self) -> EvalSeverity;
     fn short_name(&self) -> &'static str;
 }
 
@@ -54,7 +54,7 @@ pub struct Lint {
     pub short_name: String,
     /// Is this code highly-likely to be wrong, rather
     /// than merely stylistically non-ideal.
-    pub serious: bool,
+    pub severity: EvalSeverity,
     /// A description of the underlying problem.
     pub problem: String,
     /// The source code at [`location`](Lint::location).
@@ -87,7 +87,7 @@ impl<T: LintWarning> LintT<T> {
         Lint {
             location: self.location,
             short_name: self.problem.short_name().to_owned(),
-            serious: self.problem.is_serious(),
+            severity: self.problem.severity(),
             problem: self.problem.to_string(),
             original: self.original,
         }
@@ -200,12 +200,7 @@ impl From<Lint> for EvalMessage {
         Self {
             path: x.location.filename().to_owned(),
             span: Some(x.location.resolve_span()),
-            severity: if x.serious {
-                EvalSeverity::Warning
-            } else {
-                // Start with all non-serious errors disabled, and ramp up from there
-                EvalSeverity::Disabled
-            },
+            severity: x.severity,
             name: x.short_name,
             description: x.problem,
             full_error_with_span: None,

starlark/src/analysis/underscore.rs

@@ -21,6 +21,7 @@ use thiserror::Error;
 
 use crate::analysis::types::LintT;
 use crate::analysis::types::LintWarning;
+use crate::analysis::EvalSeverity;
 use crate::codemap::CodeMap;
 use crate::syntax::ast::Assign;
 use crate::syntax::ast::AstExpr;
@@ -39,8 +40,8 @@ pub(crate) enum UnderscoreWarning {
 }
 
 impl LintWarning for UnderscoreWarning {
-    fn is_serious(&self) -> bool {
-        false
+    fn severity(&self) -> EvalSeverity {
+        EvalSeverity::Disabled
     }
 
     fn short_name(&self) -> &'static str {