feat: add inconsistent mode result warning

ahabhgk · May 8, 2024 · 80c75c1 · 80c75c1
1 parent ee45a3f
commit 80c75c1
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 46 deletions.
diff --git a/src/dependencies.rs b/src/dependencies.rs
@@ -96,6 +96,10 @@ impl BalancedStack {
         self.0.is_empty()
     }
 
+    pub fn is_root(&self) -> bool {
+        self.0.len() == 1
+    }
+
     pub fn push(&mut self, item: BalancedItem, mode_data: Option<&mut ModeData>) {
         if let Some(mode_data) = mode_data {
             if matches!(
@@ -261,6 +265,7 @@ pub struct ModeData {
     default: Mode,
     current: Mode,
     property: Mode,
+    resulting_global: Option<Pos>,
 }
 
 impl ModeData {
@@ -269,6 +274,7 @@ impl ModeData {
             default,
             current: default,
             property: default,
+            resulting_global: None,
         }
     }
 
@@ -466,6 +472,7 @@ pub enum Warning<'s> {
     ExpectedUrl { range: Range, when: &'s str },
     ExpectedUrlBefore { range: Range, when: &'s str },
     ExpectedLayerBefore { range: Range, when: &'s str },
+    InconsistentModeResult { range: Range },
 }
 
 impl Display for Warning<'_> {
@@ -490,6 +497,7 @@ impl Display for Warning<'_> {
                 f,
                 "The 'layer(...)' in '{when}' should be before 'supports(...)'"
             ),
+            Warning::InconsistentModeResult { .. } => write!(f, "Inconsistent rule global/local (multiple selectors must result in the same mode for the rule)"),
         }
     }
 }
@@ -1248,7 +1256,7 @@ impl<'s, D: HandleDependency<'s>, W: HandleWarning<'s>> Visitor<'s> for LexDepen
         Some(())
     }
 
-    fn left_curly_bracket(&mut self, lexer: &mut Lexer, _: Pos, _: Pos) -> Option<()> {
+    fn left_curly_bracket(&mut self, lexer: &mut Lexer, start: Pos, _: Pos) -> Option<()> {
         match self.scope {
             Scope::TopLevel => {
                 self.allow_import_at_rule = false;
@@ -1261,6 +1269,13 @@ impl<'s, D: HandleDependency<'s>, W: HandleWarning<'s>> Visitor<'s> for LexDepen
             _ => return Some(()),
         }
         if let Some(mode_data) = &mut self.mode_data {
+            if mode_data.resulting_global.is_some() && mode_data.is_local_mode() {
+                let resulting_global_start = mode_data.resulting_global.unwrap();
+                self.handle_warning
+                    .handle_warning(Warning::InconsistentModeResult {
+                        range: Range::new(resulting_global_start, start),
+                    });
+            }
             self.balanced.update_property_mode(mode_data);
             self.balanced.pop_mode_pseudo_class(mode_data);
             self.is_next_rule_prelude = self.is_next_nested_syntax(lexer)?;
@@ -1341,17 +1356,26 @@ impl<'s, D: HandleDependency<'s>, W: HandleWarning<'s>> Visitor<'s> for LexDepen
         Some(())
     }
 
-    fn comma(&mut self, lexer: &mut Lexer<'s>, _: Pos, _: Pos) -> Option<()> {
+    fn comma(&mut self, lexer: &mut Lexer<'s>, start: Pos, _: Pos) -> Option<()> {
         let Some(mode_data) = &mut self.mode_data else {
             return Some(());
         };
-        if let Some(last) = self.balanced.last() {
-            if matches!(
-                last.kind,
-                BalancedItemKind::LocalClass | BalancedItemKind::GlobalClass
-            ) && self.balanced.len() == 1
-            {
+        if mode_data.resulting_global.is_some() && mode_data.is_local_mode() {
+            let resulting_global_start = mode_data.resulting_global.unwrap();
+            self.handle_warning
+                .handle_warning(Warning::InconsistentModeResult {
+                    range: Range::new(resulting_global_start, start),
+                });
+        }
+        if self.balanced.is_root() {
+            let last = self.balanced.last().unwrap();
+            let is_local_class = matches!(last.kind, BalancedItemKind::LocalClass);
+            let is_global_class = matches!(last.kind, BalancedItemKind::GlobalClass);
+            if is_local_class || is_global_class {
                 self.balanced.pop_mode_pseudo_class(mode_data);
+                if mode_data.resulting_global.is_none() && is_global_class {
+                    mode_data.resulting_global = Some(start);
+                }
             }
         }
         if matches!(self.scope, Scope::InBlock) && mode_data.is_property_local_mode() {

diff --git a/src/lib.rs b/src/lib.rs
@@ -1,10 +1,10 @@
 mod dependencies;
 mod lexer;
 
-pub use dependencies::Mode;
-pub use dependencies::ModeData;
 pub use dependencies::Dependency;
 pub use dependencies::LexDependencies;
+pub use dependencies::Mode;
+pub use dependencies::ModeData;
 pub use dependencies::Range;
 pub use dependencies::UrlRangeKind;
 pub use dependencies::Warning;

diff --git a/tests/postcss_plugins/modules_local_by_default_test.rs b/tests/postcss_plugins/modules_local_by_default_test.rs
@@ -1,9 +1,10 @@
-use css_module_lexer::Mode;
-use css_module_lexer::ModeData;
 use css_module_lexer::Dependency;
 use css_module_lexer::LexDependencies;
 use css_module_lexer::Lexer;
+use css_module_lexer::Mode;
+use css_module_lexer::ModeData;
 use css_module_lexer::Range;
+use css_module_lexer::Warning;
 use indoc::indoc;
 
 use crate::slice_range;
@@ -13,8 +14,9 @@ struct Options {
     mode: Mode,
 }
 
-fn modules_local_by_default(input: &str, options: Options) -> String {
+fn modules_local_by_default(input: &str, options: Options) -> (String, Vec<Warning>) {
     let mut result = String::new();
+    let mut warnings = Vec::new();
     let mut index = 0;
     let mut lexer = Lexer::from(input);
     let mut visitor = LexDependencies::new(
@@ -38,30 +40,33 @@ fn modules_local_by_default(input: &str, options: Options) -> String {
             }
             _ => {}
         },
-        |_| {},
+        |warning| warnings.push(warning),
         Some(ModeData::new(options.mode)),
     );
     lexer.lex(&mut visitor);
     let len = input.len() as u32;
     if index != len {
         result += slice_range(input, &Range::new(index, len)).unwrap();
     }
-    result
+    (result, warnings)
 }
 
 fn test(input: &str, expected: &str) {
-    let actual = modules_local_by_default(
-        input,
-        Options {
-            mode: Mode::Local,
-        },
-    );
+    let (actual, warnings) = modules_local_by_default(input, Default::default());
     assert_eq!(expected, actual);
+    assert!(warnings.is_empty());
 }
 
 fn test_with_options(input: &str, expected: &str, options: Options) {
-    let actual = modules_local_by_default(input, options);
+    let (actual, warnings) = modules_local_by_default(input, options);
     assert_eq!(expected, actual);
+    assert!(warnings.is_empty());
+}
+
+fn test_with_warning(input: &str, expected: &str, warning: &str) {
+    let (actual, warnings) = modules_local_by_default(input, Default::default());
+    assert_eq!(expected, actual);
+    assert!(warnings[0].to_string().contains(warning));
 }
 
 #[test]
@@ -551,24 +556,12 @@ fn handle_constructor_as_animation_name() {
 
 #[test]
 fn default_to_global_when_mode_provided() {
-    test_with_options(
-        ".foo {}",
-        ".foo {}",
-        Options {
-            mode: Mode::Global,
-        },
-    );
+    test_with_options(".foo {}", ".foo {}", Options { mode: Mode::Global });
 }
 
 #[test]
 fn default_to_local_when_mode_provided() {
-    test_with_options(
-        ".foo {}",
-        ":local(.foo) {}",
-        Options {
-            mode: Mode::Local,
-        },
-    );
+    test_with_options(".foo {}", ":local(.foo) {}", Options { mode: Mode::Local });
 }
 
 #[test]
@@ -598,9 +591,7 @@ fn use_correct_spacing() {
             :local(.a) .b {}
             :local(.a) .b {}
         "#},
-        Options {
-            mode: Mode::Global,
-        },
+        Options { mode: Mode::Global },
     )
 }
 
@@ -633,9 +624,7 @@ fn localize_keyframes_in_global_default_mode() {
     test_with_options(
         "@keyframes foo {}",
         "@keyframes foo {}",
-        Options {
-            mode: Mode::Global,
-        },
+        Options { mode: Mode::Global },
     );
 }
 
@@ -674,9 +663,7 @@ fn compile_in_pure_mode() {
         ":global(.foo).bar, [type=\"radio\"] ~ .label, :not(.foo), #bar {}",
         ".foo:local(.bar), [type=\"radio\"] ~ :local(.label), :not(:local(.foo)), :local(#bar) {}",
         // should be pure mode, use local before we implement it
-        Options {
-            mode: Mode::Local,
-        },
+        Options { mode: Mode::Local },
     );
 }
 
@@ -692,3 +679,11 @@ fn compile_explict_global_attribute() {
         "[type=\"radio\"], :not([type=\"radio\"]) {}",
     );
 }
+#[test]
+fn throw_on_inconsistent_selector_result() {
+    test_with_warning(
+        ":global .foo, .bar {}",
+        ".foo, :local(.bar) {}",
+        "Inconsistent",
+    );
+}
diff --git a/tests/test.rs b/tests/test.rs
@@ -154,7 +154,8 @@ fn assert_warning(input: &str, warning: &Warning, range_content: &str) {
         | Warning::NotPrecededAtImport { range }
         | Warning::ExpectedUrl { range, .. }
         | Warning::ExpectedUrlBefore { range, .. }
-        | Warning::ExpectedLayerBefore { range, .. } => {
+        | Warning::ExpectedLayerBefore { range, .. }
+        | Warning::InconsistentModeResult { range } => {
             assert_eq!(slice_range(input, range).unwrap(), range_content);
         }
     };