[move] Fix linter categories (#17558)

## Description - Fixing an issue exposed in #16819. Since Sui lints use the `lint` prefix for suppression, we need to make sure all category/code combos are distinct. - Added new lint categories, with a specific one for Sui lints ## Test plan - Updated tests --- ## Release notes Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required. For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] Indexer: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK:
MystenLabs · May 8, 2024 · 37d57d7 · 37d57d7
1 parent 6e009c2
commit 37d57d7
Show file tree

Hide file tree

Showing 21 changed files with 97 additions and 87 deletions.
diff --git a/crates/sui-move-build/src/lib.rs b/crates/sui-move-build/src/lib.rs
@@ -187,7 +187,7 @@ impl BuildConfig {
 /// (optionally report diagnostics themselves if files argument is provided).
 pub fn decorate_warnings(warning_diags: Diagnostics, files: Option<&FilesSourceText>) {
     let any_linter_warnings = warning_diags.any_with_prefix(LINT_WARNING_PREFIX);
-    let (filtered_diags_num, filtered_categories) =
+    let (filtered_diags_num, unique) =
         warning_diags.filtered_source_diags_with_prefix(LINT_WARNING_PREFIX);
     if let Some(f) = files {
         report_warnings(f, warning_diags);
@@ -196,7 +196,7 @@ pub fn decorate_warnings(warning_diags: Diagnostics, files: Option<&FilesSourceT
         eprintln!("Please report feedback on the linter warnings at https://forums.sui.io\n");
     }
     if filtered_diags_num > 0 {
-        eprintln!("Total number of linter warnings suppressed: {filtered_diags_num} (filtered categories: {filtered_categories})");
+        eprintln!("Total number of linter warnings suppressed: {filtered_diags_num} (unique lints: {unique})");
     }
 }
 

diff --git a/crates/sui/tests/cli_tests.rs b/crates/sui/tests/cli_tests.rs
@@ -2994,24 +2994,29 @@ async fn test_get_owned_objects_owned_by_address_and_check_pagination() -> Resul
 
 #[tokio::test]
 async fn test_linter_suppression_stats() -> Result<(), anyhow::Error> {
-    const LINTER_MSG: &str =
-        "Total number of linter warnings suppressed: 5 (filtered categories: 3)";
+    const LINTER_MSG: &str = "Total number of linter warnings suppressed: 5 (unique lints: 3)";
     let mut cmd = assert_cmd::Command::cargo_bin("sui").unwrap();
     let args = vec!["move", "test", "--path", "tests/data/linter"];
     let output = cmd
         .args(&args)
         .output()
         .expect("failed to run 'sui move test'");
     let out_str = str::from_utf8(&output.stderr).unwrap();
-    assert!(out_str.contains(LINTER_MSG));
+    assert!(
+        out_str.contains(LINTER_MSG),
+        "Expected to match {LINTER_MSG}, got: {out_str}"
+    );
     // test no-lint suppresses
     let args = vec!["move", "test", "--no-lint", "--path", "tests/data/linter"];
     let output = cmd
         .args(&args)
         .output()
         .expect("failed to run 'sui move test'");
     let out_str = str::from_utf8(&output.stderr).unwrap();
-    assert!(!out_str.contains(LINTER_MSG));
+    assert!(
+        !out_str.contains(LINTER_MSG),
+        "Expected _not to_ match {LINTER_MSG}, got: {out_str}"
+    );
     Ok(())
 }
 

diff --git a/external-crates/move/crates/move-compiler/src/diagnostics/mod.rs b/external-crates/move/crates/move-compiler/src/diagnostics/mod.rs
@@ -660,9 +660,8 @@ impl Diagnostics {
             .any(|d| d.info().category() == Category::Syntax as u8 && d.primary_label.0 == loc)
     }
 
-    /// Returns the number of diags filtered in source (user) code (an not in the dependencies) that
-    /// have a given prefix (first value returned) and how many different categories of diags were
-    /// filtered.
+    /// Returns the number of diags filtered in source (user) code (not in the dependencies) that
+    /// have a given prefix and how many different unique lints were filtered.
     pub fn filtered_source_diags_with_prefix(&self, prefix: &str) -> (usize, usize) {
         let Self {
             diags: Some(inner),
@@ -672,14 +671,14 @@ impl Diagnostics {
             return (0, 0);
         };
         let mut filtered_diags_num = 0;
-        let mut filtered_categories = HashSet::new();
+        let mut unique = HashSet::new();
         inner.filtered_source_diagnostics.iter().for_each(|d| {
             if d.info.external_prefix() == Some(prefix) {
                 filtered_diags_num += 1;
-                filtered_categories.insert(d.info.category());
+                unique.insert((d.info.category(), d.info.code()));
             }
         });
-        (filtered_diags_num, filtered_categories.len())
+        (filtered_diags_num, unique.len())
     }
 
     fn env_color(&self) -> ColorChoice {

diff --git a/external-crates/move/crates/move-compiler/src/linters/mod.rs b/external-crates/move/crates/move-compiler/src/linters/mod.rs
@@ -15,6 +15,17 @@ pub enum LintLevel {
     All,
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[repr(u8)]
+pub enum LinterDiagnosticCategory {
+    Correctness,
+    Complexity,
+    Suspicious,
+    Deprecated,
+    Style,
+    Sui = 99,
+}
+
 pub const ALLOW_ATTR_CATEGORY: &str = "lint";
 pub const LINT_WARNING_PREFIX: &str = "Lint ";
 

diff --git a/external-crates/move/crates/move-compiler/src/sui_mode/linters/coin_field.rs b/external-crates/move/crates/move-compiler/src/sui_mode/linters/coin_field.rs
@@ -15,15 +15,15 @@ use move_ir_types::location::Loc;
 use move_symbol_pool::Symbol;
 
 use super::{
-    LinterDiagCategory, COIN_MOD_NAME, COIN_STRUCT_NAME, LINTER_DEFAULT_DIAG_CODE,
+    LinterDiagnosticCategory, LinterDiagnosticCode, COIN_MOD_NAME, COIN_STRUCT_NAME,
     LINT_WARNING_PREFIX, SUI_PKG_NAME,
 };
 
 const COIN_FIELD_DIAG: DiagnosticInfo = custom(
     LINT_WARNING_PREFIX,
     Severity::Warning,
-    LinterDiagCategory::CoinField as u8,
-    LINTER_DEFAULT_DIAG_CODE,
+    LinterDiagnosticCategory::Sui as u8,
+    LinterDiagnosticCode::CoinField as u8,
     "sub-optimal 'sui::coin::Coin' field type",
 );
 

diff --git a/external-crates/move/crates/move-compiler/src/sui_mode/linters/collection_equality.rs b/external-crates/move/crates/move-compiler/src/sui_mode/linters/collection_equality.rs
@@ -21,8 +21,8 @@ use crate::{
 };
 
 use super::{
-    base_type, LinterDiagCategory, BAG_MOD_NAME, BAG_STRUCT_NAME, LINKED_TABLE_MOD_NAME,
-    LINKED_TABLE_STRUCT_NAME, LINTER_DEFAULT_DIAG_CODE, LINT_WARNING_PREFIX, OBJECT_BAG_MOD_NAME,
+    base_type, LinterDiagnosticCategory, LinterDiagnosticCode, BAG_MOD_NAME, BAG_STRUCT_NAME,
+    LINKED_TABLE_MOD_NAME, LINKED_TABLE_STRUCT_NAME, LINT_WARNING_PREFIX, OBJECT_BAG_MOD_NAME,
     OBJECT_BAG_STRUCT_NAME, OBJECT_TABLE_MOD_NAME, OBJECT_TABLE_STRUCT_NAME, SUI_PKG_NAME,
     TABLE_MOD_NAME, TABLE_STRUCT_NAME, TABLE_VEC_MOD_NAME, TABLE_VEC_STRUCT_NAME, VEC_MAP_MOD_NAME,
     VEC_MAP_STRUCT_NAME, VEC_SET_MOD_NAME, VEC_SET_STRUCT_NAME,
@@ -31,8 +31,8 @@ use super::{
 const COLLECTIONS_EQUALITY_DIAG: DiagnosticInfo = custom(
     LINT_WARNING_PREFIX,
     Severity::Warning,
-    LinterDiagCategory::CollectionEquality as u8,
-    LINTER_DEFAULT_DIAG_CODE,
+    LinterDiagnosticCategory::Sui as u8,
+    LinterDiagnosticCode::CollectionEquality as u8,
     "possibly useless collections compare",
 );
 

diff --git a/external-crates/move/crates/move-compiler/src/sui_mode/linters/custom_state_change.rs b/external-crates/move/crates/move-compiler/src/sui_mode/linters/custom_state_change.rs
@@ -33,7 +33,7 @@ use crate::{
 use std::collections::BTreeMap;
 
 use super::{
-    LinterDiagCategory, FREEZE_FUN, INVALID_LOC, LINTER_DEFAULT_DIAG_CODE, LINT_WARNING_PREFIX,
+    LinterDiagnosticCategory, LinterDiagnosticCode, FREEZE_FUN, INVALID_LOC, LINT_WARNING_PREFIX,
     RECEIVE_FUN, SHARE_FUN, SUI_PKG_NAME, TRANSFER_FUN, TRANSFER_MOD_NAME,
 };
 
@@ -47,8 +47,8 @@ const PRIVATE_OBJ_FUNCTIONS: &[(&str, &str, &str)] = &[
 const CUSTOM_STATE_CHANGE_DIAG: DiagnosticInfo = custom(
     LINT_WARNING_PREFIX,
     Severity::Warning,
-    LinterDiagCategory::CustomStateChange as u8,
-    LINTER_DEFAULT_DIAG_CODE,
+    LinterDiagnosticCategory::Sui as u8,
+    LinterDiagnosticCode::CustomStateChange as u8,
     "potentially unenforceable custom transfer/share/freeze policy",
 );
 

diff --git a/external-crates/move/crates/move-compiler/src/sui_mode/linters/freeze_wrapped.rs b/external-crates/move/crates/move-compiler/src/sui_mode/linters/freeze_wrapped.rs
@@ -26,15 +26,15 @@ use move_ir_types::location::*;
 use move_symbol_pool::Symbol;
 
 use super::{
-    base_type, LinterDiagCategory, FREEZE_FUN, LINTER_DEFAULT_DIAG_CODE, LINT_WARNING_PREFIX,
+    base_type, LinterDiagnosticCategory, LinterDiagnosticCode, FREEZE_FUN, LINT_WARNING_PREFIX,
     PUBLIC_FREEZE_FUN, SUI_PKG_NAME, TRANSFER_MOD_NAME,
 };
 
 const FREEZE_WRAPPING_DIAG: DiagnosticInfo = custom(
     LINT_WARNING_PREFIX,
     Severity::Warning,
-    LinterDiagCategory::FreezeWrapped as u8,
-    LINTER_DEFAULT_DIAG_CODE,
+    LinterDiagnosticCategory::Sui as u8,
+    LinterDiagnosticCode::FreezeWrapped as u8,
     "attempting to freeze wrapped objects",
 );
 

diff --git a/external-crates/move/crates/move-compiler/src/sui_mode/linters/mod.rs b/external-crates/move/crates/move-compiler/src/sui_mode/linters/mod.rs
@@ -7,7 +7,7 @@ use crate::{
     diagnostics::codes::WarningFilter,
     expansion::ast as E,
     hlir::ast::{BaseType_, SingleType, SingleType_},
-    linters::{LintLevel, ALLOW_ATTR_CATEGORY, LINT_WARNING_PREFIX},
+    linters::{LintLevel, LinterDiagnosticCategory, ALLOW_ATTR_CATEGORY, LINT_WARNING_PREFIX},
     naming::ast as N,
     typing::visitor::TypingVisitor,
 };
@@ -75,7 +75,8 @@ pub const RANDOM_GENERATOR_STRUCT_NAME: &str = "RandomGenerator";
 
 pub const INVALID_LOC: Loc = Loc::invalid();
 
-pub enum LinterDiagCategory {
+#[repr(u8)]
+pub enum LinterDiagnosticCode {
     ShareOwned,
     SelfTransfer,
     CustomStateChange,
@@ -85,53 +86,49 @@ pub enum LinterDiagCategory {
     PublicRandom,
 }
 
-/// A default code for each linter category (as long as only one code per category is used, no other
-/// codes are needed, otherwise they should be defined to be unique per-category).
-pub const LINTER_DEFAULT_DIAG_CODE: u8 = 1;
-
 pub fn known_filters() -> (Option<Symbol>, Vec<WarningFilter>) {
     let filters = vec![
         WarningFilter::All(Some(LINT_WARNING_PREFIX)),
         WarningFilter::code(
             Some(LINT_WARNING_PREFIX),
-            LinterDiagCategory::ShareOwned as u8,
-            LINTER_DEFAULT_DIAG_CODE,
+            LinterDiagnosticCategory::Sui as u8,
+            LinterDiagnosticCode::ShareOwned as u8,
             Some(SHARE_OWNED_FILTER_NAME),
         ),
         WarningFilter::code(
             Some(LINT_WARNING_PREFIX),
-            LinterDiagCategory::SelfTransfer as u8,
-            LINTER_DEFAULT_DIAG_CODE,
+            LinterDiagnosticCategory::Sui as u8,
+            LinterDiagnosticCode::SelfTransfer as u8,
             Some(SELF_TRANSFER_FILTER_NAME),
         ),
         WarningFilter::code(
             Some(LINT_WARNING_PREFIX),
-            LinterDiagCategory::CustomStateChange as u8,
-            LINTER_DEFAULT_DIAG_CODE,
+            LinterDiagnosticCategory::Sui as u8,
+            LinterDiagnosticCode::CustomStateChange as u8,
             Some(CUSTOM_STATE_CHANGE_FILTER_NAME),
         ),
         WarningFilter::code(
             Some(LINT_WARNING_PREFIX),
-            LinterDiagCategory::CoinField as u8,
-            LINTER_DEFAULT_DIAG_CODE,
+            LinterDiagnosticCategory::Sui as u8,
+            LinterDiagnosticCode::CoinField as u8,
             Some(COIN_FIELD_FILTER_NAME),
         ),
         WarningFilter::code(
             Some(LINT_WARNING_PREFIX),
-            LinterDiagCategory::FreezeWrapped as u8,
-            LINTER_DEFAULT_DIAG_CODE,
+            LinterDiagnosticCategory::Sui as u8,
+            LinterDiagnosticCode::FreezeWrapped as u8,
             Some(FREEZE_WRAPPED_FILTER_NAME),
         ),
         WarningFilter::code(
             Some(LINT_WARNING_PREFIX),
-            LinterDiagCategory::CollectionEquality as u8,
-            LINTER_DEFAULT_DIAG_CODE,
+            LinterDiagnosticCategory::Sui as u8,
+            LinterDiagnosticCode::CollectionEquality as u8,
             Some(COLLECTION_EQUALITY_FILTER_NAME),
         ),
         WarningFilter::code(
             Some(LINT_WARNING_PREFIX),
-            LinterDiagCategory::PublicRandom as u8,
-            LINTER_DEFAULT_DIAG_CODE,
+            LinterDiagnosticCategory::Sui as u8,
+            LinterDiagnosticCode::PublicRandom as u8,
             Some(PUBLIC_RANDOM_FILTER_NAME),
         ),
     ];

diff --git a/external-crates/move/crates/move-compiler/src/sui_mode/linters/public_random.rs b/external-crates/move/crates/move-compiler/src/sui_mode/linters/public_random.rs
@@ -18,15 +18,15 @@ use crate::{
 };
 
 use super::{
-    LinterDiagCategory, LINTER_DEFAULT_DIAG_CODE, LINT_WARNING_PREFIX,
+    LinterDiagnosticCategory, LinterDiagnosticCode, LINT_WARNING_PREFIX,
     RANDOM_GENERATOR_STRUCT_NAME, RANDOM_MOD_NAME, RANDOM_STRUCT_NAME, SUI_PKG_NAME,
 };
 
 const PUBLIC_RANDOM_DIAG: DiagnosticInfo = custom(
     LINT_WARNING_PREFIX,
     Severity::Warning,
-    LinterDiagCategory::PublicRandom as u8,
-    LINTER_DEFAULT_DIAG_CODE,
+    LinterDiagnosticCategory::Sui as u8,
+    LinterDiagnosticCode::PublicRandom as u8,
     "Risky use of 'sui::random'",
 );
 

diff --git a/external-crates/move/crates/move-compiler/src/sui_mode/linters/self_transfer.rs b/external-crates/move/crates/move-compiler/src/sui_mode/linters/self_transfer.rs
@@ -27,8 +27,8 @@ use crate::{
 use std::collections::BTreeMap;
 
 use super::{
-    type_abilities, LinterDiagCategory, INVALID_LOC, LINTER_DEFAULT_DIAG_CODE, LINT_WARNING_PREFIX,
-    PUBLIC_TRANSFER_FUN, SUI_PKG_NAME, TRANSFER_FUN, TRANSFER_MOD_NAME,
+    type_abilities, LinterDiagnosticCategory, LinterDiagnosticCode, INVALID_LOC,
+    LINT_WARNING_PREFIX, PUBLIC_TRANSFER_FUN, SUI_PKG_NAME, TRANSFER_FUN, TRANSFER_MOD_NAME,
 };
 
 const TRANSFER_FUNCTIONS: &[(&str, &str, &str)] = &[
@@ -39,8 +39,8 @@ const TRANSFER_FUNCTIONS: &[(&str, &str, &str)] = &[
 const SELF_TRANSFER_DIAG: DiagnosticInfo = custom(
     LINT_WARNING_PREFIX,
     Severity::Warning,
-    LinterDiagCategory::SelfTransfer as u8,
-    LINTER_DEFAULT_DIAG_CODE,
+    LinterDiagnosticCategory::Sui as u8,
+    LinterDiagnosticCode::SelfTransfer as u8,
     "non-composable transfer to sender",
 );
 

diff --git a/external-crates/move/crates/move-compiler/src/sui_mode/linters/share_owned.rs b/external-crates/move/crates/move-compiler/src/sui_mode/linters/share_owned.rs
@@ -30,7 +30,7 @@ use crate::{
 use std::collections::BTreeMap;
 
 use super::{
-    type_abilities, LinterDiagCategory, LINTER_DEFAULT_DIAG_CODE, LINT_WARNING_PREFIX,
+    type_abilities, LinterDiagnosticCategory, LinterDiagnosticCode, LINT_WARNING_PREFIX,
     PUBLIC_SHARE_FUN, SHARE_FUN, SUI_PKG_NAME, TRANSFER_MOD_NAME,
 };
 
@@ -42,8 +42,8 @@ const SHARE_FUNCTIONS: &[(&str, &str, &str)] = &[
 const SHARE_OWNED_DIAG: DiagnosticInfo = custom(
     LINT_WARNING_PREFIX,
     Severity::Warning,
-    LinterDiagCategory::ShareOwned as u8,
-    LINTER_DEFAULT_DIAG_CODE,
+    LinterDiagnosticCategory::Sui as u8,
+    LinterDiagnosticCode::ShareOwned as u8,
     "possible owned object share",
 );
 

diff --git a/external-crates/move/crates/move-compiler/tests/sui_mode/linter/coin_field.exp b/external-crates/move/crates/move-compiler/tests/sui_mode/linter/coin_field.exp
@@ -1,4 +1,4 @@
-warning[Lint W03001]: sub-optimal 'sui::coin::Coin' field type
+warning[Lint W99003]: sub-optimal 'sui::coin::Coin' field type
    ┌─ tests/sui_mode/linter/coin_field.move:11:12
    │
 11 │     struct S2 has key, store {
@@ -9,7 +9,7 @@ warning[Lint W03001]: sub-optimal 'sui::coin::Coin' field type
    │
    = This warning can be suppressed with '#[allow(lint(coin_field))]' applied to the 'module' or module member ('const', 'fun', or 'struct')
 
-warning[Lint W03001]: sub-optimal 'sui::coin::Coin' field type
+warning[Lint W99003]: sub-optimal 'sui::coin::Coin' field type
    ┌─ tests/sui_mode/linter/coin_field.move:25:12
    │
 25 │     struct S2 has key, store {