filter diagnostics

starkware-libs · Jul 10, 2024 · c9c3d12 · c9c3d12
1 parent 5aacae9
commit c9c3d12
Show file tree

Hide file tree

Showing 13 changed files with 54 additions and 106 deletions.
diff --git a/crates/cairo-lang-diagnostics/src/diagnostics.rs b/crates/cairo-lang-diagnostics/src/diagnostics.rs
@@ -46,6 +46,9 @@ pub trait DiagnosticEntry: Clone + fmt::Debug + Eq + Hash {
     fn error_code(&self) -> Option<ErrorCode> {
         None
     }
+    /// Returns true if the two should be regarded as the same kind when filtering duplicate
+    /// diagnostics.
+    fn is_same_kind(&self, _other: &Self) -> bool;
 
     // TODO(spapini): Add a way to inspect the diagnostic programmatically, e.g, downcast.
 }
@@ -256,8 +259,7 @@ impl<TEntry: DiagnosticEntry> Diagnostics<TEntry> {
         let mut res: Vec<FormattedDiagnosticEntry> = Vec::new();
 
         let files_db = db.upcast();
-        // Format leaves.
-        for entry in &self.0.leaves {
+        for entry in &self.get_diagnostics_without_duplicates(db) {
             let mut msg = String::new();
             msg += &format_diagnostics(files_db, &entry.format(db), entry.location(db));
             for note in entry.notes(db) {
@@ -269,8 +271,6 @@ impl<TEntry: DiagnosticEntry> Diagnostics<TEntry> {
                 FormattedDiagnosticEntry::new(entry.severity(), entry.error_code(), msg);
             res.push(formatted);
         }
-        // Format subtrees.
-        res.extend(self.0.subtrees.iter().flat_map(|subtree| subtree.format_with_severity(db)));
         res
     }
 
@@ -296,13 +296,45 @@ impl<TEntry: DiagnosticEntry> Diagnostics<TEntry> {
     }
 
     // TODO(spapini): This is temporary. Remove once the logic in language server doesn't use this.
+    /// Get all diagnostics.
     pub fn get_all(&self) -> Vec<TEntry> {
         let mut res = self.0.leaves.clone();
         for subtree in &self.0.subtrees {
             res.extend(subtree.get_all())
         }
         res
     }
+
+    /// Get diagnostics without duplication.
+    ///
+    /// Two diagnostics are considered duplicated if both point to
+    /// the same location in the user code, and are of the same kind.
+    pub fn get_diagnostics_without_duplicates(&self, db: &TEntry::DbType) -> Vec<TEntry> {
+        let diagnostic_with_dup = self.get_all();
+        if diagnostic_with_dup.is_empty() {
+            return diagnostic_with_dup;
+        }
+        let files_db = db.upcast();
+        let mut indexed_dup_diagnostic = diagnostic_with_dup
+            .iter()
+            .enumerate()
+            .sorted_by_key(|(idx, diag)| (diag.location(db).user_location(files_db).span, *idx));
+        let mut prev_diagnostic_indexed = indexed_dup_diagnostic.next().unwrap();
+        let mut diagnostic_without_dup = vec![prev_diagnostic_indexed];
+
+        for diag in indexed_dup_diagnostic {
+            if prev_diagnostic_indexed.1.is_same_kind(diag.1)
+                && prev_diagnostic_indexed.1.location(db).user_location(files_db).span
+                    == diag.1.location(db).user_location(files_db).span
+            {
+                continue;
+            }
+            diagnostic_without_dup.push(diag);
+            prev_diagnostic_indexed = diag;
+        }
+        diagnostic_without_dup.sort_by_key(|(idx, _)| *idx);
+        diagnostic_without_dup.into_iter().map(|(_, diag)| diag.clone()).collect()
+    }
 }
 impl<TEntry: DiagnosticEntry> Default for Diagnostics<TEntry> {
     fn default() -> Self {

diff --git a/crates/cairo-lang-diagnostics/src/diagnostics_test.rs b/crates/cairo-lang-diagnostics/src/diagnostics_test.rs
@@ -29,6 +29,10 @@ impl DiagnosticEntry for SimpleDiag {
             },
         }
     }
+
+    fn is_same_kind(&self, _other: &Self) -> bool {
+        true
+    }
 }
 
 fn setup() -> (FilesDatabaseForTesting, FileId) {

diff --git a/crates/cairo-lang-filesystem/src/span.rs b/crates/cairo-lang-filesystem/src/span.rs
@@ -67,7 +67,7 @@ impl Sub for TextOffset {
 }
 
 /// A range of text offsets that form a span (like text selection).
-#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, Hash)]
+#[derive(Copy, Clone, Debug, Default, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub struct TextSpan {
     pub start: TextOffset,
     pub end: TextOffset,

diff --git a/crates/cairo-lang-language-server/src/lang/diagnostics/lsp.rs b/crates/cairo-lang-language-server/src/lang/diagnostics/lsp.rs
@@ -15,7 +15,7 @@ pub fn map_cairo_diagnostics_to_lsp<T: DiagnosticEntry>(
     diags: &mut Vec<Diagnostic>,
     diagnostics: &Diagnostics<T>,
 ) {
-    for diagnostic in diagnostics.get_all() {
+    for diagnostic in diagnostics.get_diagnostics_without_duplicates(db) {
         let mut message = diagnostic.format(db);
         let mut related_information = vec![];
         for note in diagnostic.notes(db) {

diff --git a/crates/cairo-lang-lowering/src/diagnostic.rs b/crates/cairo-lang-lowering/src/diagnostic.rs
@@ -101,6 +101,10 @@ impl DiagnosticEntry for LoweringDiagnostic {
         }
         self.location.stable_location.diagnostic_location(db.upcast())
     }
+
+    fn is_same_kind(&self, _other: &Self) -> bool {
+        _other.kind == self.kind
+    }
 }
 
 impl MatchError {

diff --git a/crates/cairo-lang-parser/src/diagnostic.rs b/crates/cairo-lang-parser/src/diagnostic.rs
@@ -130,4 +130,8 @@ Did you mean to write `{identifier}!{left}...{right}'?",
     fn location(&self, _db: &dyn FilesGroup) -> cairo_lang_diagnostics::DiagnosticLocation {
         cairo_lang_diagnostics::DiagnosticLocation { file_id: self.file_id, span: self.span }
     }
+
+    fn is_same_kind(&self, _other: &Self) -> bool {
+        _other.kind == self.kind
+    }
 }
diff --git a/crates/cairo-lang-semantic/src/diagnostic.rs b/crates/cairo-lang-semantic/src/diagnostic.rs
@@ -861,6 +861,10 @@ impl DiagnosticEntry for SemanticDiagnostic {
     fn error_code(&self) -> Option<ErrorCode> {
         self.kind.error_code()
     }
+
+    fn is_same_kind(&self, _other: &Self) -> bool {
+        _other.kind == self.kind
+    }
 }
 
 #[derive(Clone, Debug, Eq, Hash, PartialEq)]

diff --git a/crates/cairo-lang-starknet/src/plugin/plugin_test_data/components/component b/crates/cairo-lang-starknet/src/plugin/plugin_test_data/components/component
@@ -1091,13 +1091,3 @@ warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[featur
  --> lib.cairo:14:14
         map: LegacyMap<u32, u32>,
              ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:14:14
-        map: LegacyMap<u32, u32>,
-             ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:14:14
-        map: LegacyMap<u32, u32>,
-             ^*******^
diff --git a/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/diagnostics b/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/diagnostics
@@ -190,11 +190,6 @@ warning: Plugin diagnostic: Failed to generate ABI: Entrypoints must have a self
 #[starknet::contract]
 ^*******************^
 
-warning: Plugin diagnostic: Failed to generate ABI: Entrypoints must have a self first param.
- --> lib.cairo:1:1
-#[starknet::contract]
-^*******************^
-
 //! > ==========================================================================
 
 //! > Test invalid constructor name.

diff --git a/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/new_storage_interface b/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/new_storage_interface
@@ -2609,13 +2609,3 @@ warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[featur
  --> lib.cairo:63:28
         legacy_map_balace: LegacyMap<usize, usize>,
                            ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:63:28
-        legacy_map_balace: LegacyMap<usize, usize>,
-                           ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:63:28
-        legacy_map_balace: LegacyMap<usize, usize>,
-                           ^*******^
diff --git a/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/storage b/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/storage
@@ -425,53 +425,3 @@ warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[featur
  --> lib.cairo:24:35
         inner_type_to_inner_type: LegacyMap::<inner::InnerType, inner::InnerType>,
                                   ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:18:26
-        felt252_to_u128: LegacyMap::<felt252, u128>,
-                         ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:19:23
-        u128_to_bool: LegacyMap::<u128, bool>,
-                      ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:20:26
-        bool_to_felt252: LegacyMap::<bool, felt252>,
-                         ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:22:35
-        outer_type_to_outer_type: LegacyMap::<super::OuterType, super::OuterType>,
-                                  ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:24:35
-        inner_type_to_inner_type: LegacyMap::<inner::InnerType, inner::InnerType>,
-                                  ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:18:26
-        felt252_to_u128: LegacyMap::<felt252, u128>,
-                         ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:19:23
-        u128_to_bool: LegacyMap::<u128, bool>,
-                      ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:20:26
-        bool_to_felt252: LegacyMap::<bool, felt252>,
-                         ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:22:35
-        outer_type_to_outer_type: LegacyMap::<super::OuterType, super::OuterType>,
-                                  ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:24:35
-        inner_type_to_inner_type: LegacyMap::<inner::InnerType, inner::InnerType>,
-                                  ^*******^
diff --git a/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/user_defined_types b/crates/cairo-lang-starknet/src/plugin/plugin_test_data/contracts/user_defined_types
@@ -723,23 +723,3 @@ warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[featur
  --> lib.cairo:11:28
         zero_size_mapping: LegacyMap::<ZeroSize, ZeroSize>,
                            ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:10:18
-        mapping: LegacyMap::<WrappedFelt252, WrappedFelt252>,
-                 ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:11:28
-        zero_size_mapping: LegacyMap::<ZeroSize, ZeroSize>,
-                           ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:10:18
-        mapping: LegacyMap::<WrappedFelt252, WrappedFelt252>,
-                 ^*******^
-
-warning: Usage of deprecated feature `"deprecated_legacy_map"` with no `#[feature("deprecated_legacy_map")]` attribute. Note: "Use `starknet::storage::Map` instead."
- --> lib.cairo:11:28
-        zero_size_mapping: LegacyMap::<ZeroSize, ZeroSize>,
-                           ^*******^
diff --git a/crates/cairo-lang-starknet/src/test_data/abi_failures b/crates/cairo-lang-starknet/src/test_data/abi_failures
@@ -33,11 +33,6 @@ error: Trait has no implementation in context: core::starknet::event::Event::<te
     #[derive(Drop, starknet::Event)]
                    ^*************^
 
-error: Trait has no implementation in context: core::starknet::event::Event::<test::A>.
- --> lib.cairo:10:20
-    #[derive(Drop, starknet::Event)]
-                   ^*************^
-
 //! > ==========================================================================
 
 //! > Test flat event variant which is not an enum.