Refactor variable search to match regex in atRules params and declara…

…tion values. This improves findings reliability by eliminating the need to list all edge cases.
stylelint-scss · Mar 1, 2024 · 64d7fc2 · 64d7fc2
1 parent e4e8a02
commit 64d7fc2
Show file tree

Hide file tree

Showing 2 changed files with 112 additions and 50 deletions.
diff --git a/src/rules/no-unused-private-members/__tests__/index.js b/src/rules/no-unused-private-members/__tests__/index.js
@@ -96,10 +96,11 @@ testRule({
     },
     {
       code: `
-      $-radius: 3px;
+      $-a: 3px;
+      $_b: 1px;
       
       .action-buttons {
-        margin: map.get($-radius);
+        margin: $-a map.get($_b);
       }
       `,
       description: "map.get uses variable"
@@ -119,6 +120,55 @@ testRule({
           );
       }`,
       description: "Is in theme declaration"
+    },
+    {
+      code: `
+      $_app-bar-height: 65px;
+      $_header-height: 70px;
+      $_explorer-margin: 64px;
+      $_total-offset: $_app-bar-height + $_header-height + $_explorer-margin;
+
+      . b {
+        margin: $_total-offset;
+      }
+      `,
+      description: "Is in theme declaration"
+    },
+    {
+      code: `
+      @function _add-one($n1) { @return $n1 + 1 }
+      @function _add-two($n1) { @return $n1 + 2 }
+      @function _add-three($n1) {
+      @return _add-two (
+        _add-one($n1)
+      )}
+      
+      .b {
+        margin: _add-three(10);
+      }`,
+      description: "Is in theme declaration"
+    },
+    {
+      code: `
+      $_a: false;
+
+      .btn {
+        @if ($_a) {
+          margin: 0px;
+        }
+      }`,
+      description: "Is used in @if"
+    },
+    {
+      code: `
+      $_gm-toolbar-item-state-offset: 0.5 * 3;
+      @mixin _position-offset($offset: $_gm-toolbar-item-state-offset) {
+        top: $offset;
+      }
+      .b {
+        @include _position-offset;
+      }`,
+      description: "Is used as a mixin default parameter"
     }
   ],
 
@@ -232,7 +282,7 @@ testRule({
       $_b: 900;
       
       .action-buttons {
-        margin: map.get($_a);
+        margin: 5px map.get($_a);
       }
       `,
       message: messages.expected("$_b"),
@@ -258,6 +308,36 @@ testRule({
       line: 3,
       column: 7,
       description: "Is in theme declaration"
+    },
+    {
+      code: `
+      $_a: false;
+      $_b: true;
+
+      .btn {
+        @if ($_a) {
+          margin: 0px;
+        }
+      }`,
+      message: messages.expected("$_b"),
+      line: 3,
+      column: 7,
+      description: "Is used in @if"
+    },
+    {
+      code: `
+      $_a: '&:hover';
+      $_b: '&:not(:disabled):active';
+
+      .@mixin states-selector() {
+        #{$_a} {
+          margin: 0px;
+        }
+      }`,
+      message: messages.expected("$_b"),
+      line: 3,
+      column: 7,
+      description: "Variable in interpolated selector."
     }
   ]
 });
diff --git a/src/rules/no-unused-private-members/index.js b/src/rules/no-unused-private-members/index.js
@@ -20,9 +20,9 @@ function extractFunctionName(inputString) {
   return matches;
 }
 
-function extractFunctionArgs(inputString) {
-  const matches = [...inputString.matchAll(/\(([^()]*)\)/g)].flat();
-  return matches.length > 0 ? matches[1].split(",") : [];
+function getPrivateMembers(inputString) {
+  const matches = inputString.match(/([$%]*[-_]+[\w\d-_]+)/g);
+  return matches;
 }
 
 function rule(primaryOption) {
@@ -98,56 +98,38 @@ function rule(primaryOption) {
       }
     });
 
-    root.walkAtRules(node => {
-      if (node.name === "extend" && privateMembers.selectors.has(node.params)) {
-        privateMembers.selectors.delete(node.params);
-      }
-
-      const param = extractFunctionName(node.params)[1]
-        ? extractFunctionName(node.params)[1]
-        : node.params;
-      if (node.name === "include" && privateMembers.mixins.has(param)) {
-        privateMembers.mixins.delete(param);
-      }
-
-      const params = extractFunctionArgs(node.params);
-      if (node.name === "include" && params) {
-        params.forEach(param => {
-          const isThemeDeclaration = param.split(":");
-          if (isThemeDeclaration.length > 1) {
-            const value = isThemeDeclaration[1].trim();
-            if (privateMembers.variables.has(value)) {
-              privateMembers.variables.delete(value);
-            }
-          }
-          if (privateMembers.variables.has(param)) {
-            privateMembers.variables.delete(param);
-          }
-        });
+    root.walk(node => {
+      if (node.type === "atrule" || node.type === "rule") {
+        const value = node.type === "rule" ? node.selector : node.params;
+        const valuePrivateMembers = getPrivateMembers(value);
+        if (valuePrivateMembers) {
+          valuePrivateMembers.forEach(privateMember => {
+            if (privateMembers.mixins.get(privateMember) !== node)
+              privateMembers.mixins.delete(privateMember);
+            if (
+              privateMembers.selectors.get(privateMember) !== node &&
+              node.type === "atrule"
+            )
+              privateMembers.selectors.delete(privateMember);
+            if (privateMembers.variables.get(privateMember) !== node)
+              privateMembers.variables.delete(privateMember);
+            if (privateMembers.functions.get(privateMember) !== node)
+              privateMembers.functions.delete(privateMember);
+          });
+        }
       }
     });
 
     root.walkDecls(decls => {
-      const hasFunctions = extractFunctionName(decls.value);
-      hasFunctions.forEach(func => {
-        if (privateMembers.functions.has(func)) {
-          privateMembers.functions.delete(func);
-        }
-      });
-
-      // Functions in values like map.get
-      const params = extractFunctionArgs(decls.value);
-      if (params) {
-        params.forEach(param => {
-          if (privateMembers.variables.has(param)) {
-            privateMembers.variables.delete(param);
-          }
+      const valuePrivateMembers = getPrivateMembers(decls.value);
+      if (valuePrivateMembers) {
+        valuePrivateMembers.forEach(privateMember => {
+          if (privateMembers.variables.get(privateMember) !== decls)
+            privateMembers.variables.delete(privateMember);
+          if (privateMembers.functions.get(privateMember) !== decls)
+            privateMembers.functions.delete(privateMember);
         });
       }
-
-      if (privateMembers.variables.has(decls.value)) {
-        privateMembers.variables.delete(decls.value);
-      }
     });
 
     for (const types in privateMembers) {