|
| 1 | +--- |
| 2 | +name: add-code-inspector-rule |
| 3 | +description: | |
| 4 | + Add a new custom CodeInspector rule to detect problematic Wolfram Language patterns. |
| 5 | + Use when asked to: "add a code inspector rule", "create an inspection rule", |
| 6 | + "add a lint rule", "detect [pattern] in code inspector", "add CodeInspector rule", |
| 7 | + "new code analysis rule". |
| 8 | +argument-hint: "description of the rule to add" |
| 9 | +--- |
| 10 | + |
| 11 | +# Add a Custom CodeInspector Rule |
| 12 | + |
| 13 | +Follow this workflow to implement a new code inspection rule. The rule system lives in `Kernel/Tools/CodeInspector/Rules.wl` with tests in `Tests/CodeInspectorTool.wlt`. |
| 14 | + |
| 15 | +## Step 1: Understand the Rule |
| 16 | + |
| 17 | +$ARGUMENTS |
| 18 | + |
| 19 | +Clarify these details (ask the user if not clear): |
| 20 | + |
| 21 | +- **What code pattern should be detected?** Get an example of the problematic code. |
| 22 | +- **Why is it problematic?** This becomes the inspection message. |
| 23 | +- **Severity:** Fatal (code will not run, parse etc.), Error (almost certainly a mistake), Warning (likely a mistake), Remark (suggestion), Formatting (style). |
| 24 | +- **Confidence:** 0.95 for highly certain rules, 0.9 for confident, 0.7-0.9 for likely. |
| 25 | + |
| 26 | +## Step 2: Choose the Rule Type |
| 27 | + |
| 28 | +| Type | When to Use | Add To | |
| 29 | +|------|-------------|--------| |
| 30 | +| **Abstract** | Match simplified AST patterns (most common) | `$customAbstractRules` in Rules.wl | |
| 31 | +| **Concrete** | Need whitespace/comments (e.g., comment content) | `$concreteRules` in Rules.wl | |
| 32 | +| **Aggregate** | Analyze relationships between multiple AST nodes | `$aggregateRules` in Rules.wl | |
| 33 | +| **Text-level** | Raw source text (line length, file size, etc.) | `textLevelInspections` function in Rules.wl | |
| 34 | + |
| 35 | +## Step 3: Explore the AST Structure |
| 36 | + |
| 37 | +Use the `WolframLanguageEvaluator` tool to parse example code and understand its AST: |
| 38 | + |
| 39 | +```wl |
| 40 | +Needs["CodeParser`"]; |
| 41 | +(* For abstract rules: *) |
| 42 | +CodeParser`CodeParse["problematic code here"] |
| 43 | +(* For concrete rules: *) |
| 44 | +CodeParser`CodeConcreteParse["problematic code here"] |
| 45 | +``` |
| 46 | + |
| 47 | +Study the output to determine which node types and patterns to match. Key node types: |
| 48 | +- ``CodeParser`CallNode`` — function calls like `f[x]` |
| 49 | +- ``CodeParser`LeafNode`` — atoms like `42`, `"hello"`, `Symbol` |
| 50 | +- ``CodeParser`InfixNode`` — infix ops like `a + b` |
| 51 | +- ``CodeParser`PrefixNode`` — prefix ops like `-x` |
| 52 | + |
| 53 | +**Note:** If you want to use symbols defined in `Rules.wl` in the WolframLanguageEvaluator tool during your exploration, you'll need to use their fully qualified names: |
| 54 | + |
| 55 | +```wl |
| 56 | +PacletDirectoryLoad["path/to/MCPServer"]; |
| 57 | +Get["Wolfram`MCPServer`"]; |
| 58 | +Wolfram`MCPServer`Tools`CodeInspector`Private`astPattern[...] |
| 59 | +``` |
| 60 | + |
| 61 | +## Step 4: Read the Current Rules File |
| 62 | + |
| 63 | +Read `Kernel/Tools/CodeInspector/Rules.wl` to understand the current state and find the right insertion points. |
| 64 | + |
| 65 | +## Step 5: Define Reusable Patterns (if needed) |
| 66 | + |
| 67 | +If the rule needs reusable patterns, add them to the **Argument Patterns** section (near the top of Rules.wl, after the `Needs` statements): |
| 68 | + |
| 69 | +```wl |
| 70 | +$$myPattern = HoldPattern @ Alternatives[ |
| 71 | + _SomeFunction, |
| 72 | + _AnotherFunction, |
| 73 | + SomeSymbol |
| 74 | +]; |
| 75 | +``` |
| 76 | + |
| 77 | +## Step 6: Add the Rule Entry |
| 78 | + |
| 79 | +### For abstract rules — add to `$customAbstractRules`: |
| 80 | + |
| 81 | +```wl |
| 82 | +$customAbstractRules := $customAbstractRules = <| |
| 83 | + (* ...existing rules... *) |
| 84 | + (* MyNewRule - short description *) |
| 85 | + myPattern -> inspectMyNewRule |
| 86 | +|>; |
| 87 | +``` |
| 88 | + |
| 89 | +**Pattern approaches (choose one):** |
| 90 | + |
| 91 | +Direct AST pattern matching: |
| 92 | +```wl |
| 93 | +cp`CallNode[ cp`LeafNode[ Symbol, "FunctionName"|"System`FunctionName", _ ], { _ }, _ ] -> inspectMyRule |
| 94 | +``` |
| 95 | + |
| 96 | +Using a test predicate: |
| 97 | +```wl |
| 98 | +cp`LeafNode[ Symbol, _String? myPredicateQ, _ ] -> inspectMyRule |
| 99 | +``` |
| 100 | + |
| 101 | +Using `astPattern` for Wolfram-syntax-level patterns: |
| 102 | +```wl |
| 103 | +astPattern[ - $$yieldsDateObject ] -> inspectMyRule |
| 104 | +astPattern @ HoldPattern @ ReadString[ __, CharacterEncoding -> _, ___ ] -> inspectMyRule |
| 105 | +``` |
| 106 | + |
| 107 | +### For concrete rules — add to `$concreteRules`: |
| 108 | + |
| 109 | +```wl |
| 110 | +$concreteRules := $concreteRules = <| |
| 111 | + CodeInspector`ConcreteRules`$DefaultConcreteRules, |
| 112 | + (* ...existing rules... *) |
| 113 | + myConcretePattern -> inspectMyRule |
| 114 | +|>; |
| 115 | +``` |
| 116 | + |
| 117 | +### For text-level rules — add to `textLevelInspections`: |
| 118 | + |
| 119 | +```wl |
| 120 | +textLevelInspections[ code_String ] := |
| 121 | + Module[ { lines }, |
| 122 | + lines = StringSplit[ code, "\n", All ]; |
| 123 | + Flatten @ { |
| 124 | + inspectLineLengths @ lines, |
| 125 | + inspectFileLength @ lines, |
| 126 | + inspectMyTextRule @ lines (* Add here *) |
| 127 | + } |
| 128 | + ]; |
| 129 | +``` |
| 130 | + |
| 131 | +## Step 7: Write the Handler Function |
| 132 | + |
| 133 | +Add the handler in the **Definitions** section of Rules.wl, using the appropriate subsection marker. |
| 134 | + |
| 135 | +### Standard AST handler template: |
| 136 | + |
| 137 | +```wl |
| 138 | +(* ::***...:: *) |
| 139 | +(* ::Subsection::Closed:: *) |
| 140 | +(*inspectMyNewRule*) |
| 141 | +inspectMyNewRule // beginDefinition; |
| 142 | +
|
| 143 | +inspectMyNewRule[ pos_, ast_ ] := |
| 144 | + Enclose @ Module[ { node, as }, |
| 145 | + node = ConfirmMatch[ Extract[ ast, pos ], _[ _, _, __ ], "Node" ]; |
| 146 | + as = ConfirmBy[ node[[ 3 ]], AssociationQ, "Metadata" ]; |
| 147 | + ci`InspectionObject[ |
| 148 | + "MyRuleTag", |
| 149 | + "Description of the issue shown to the user", |
| 150 | + "Warning", |
| 151 | + <| as, ConfidenceLevel -> 0.9 |> |
| 152 | + ] |
| 153 | + ]; |
| 154 | +
|
| 155 | +inspectMyNewRule // endDefinition; |
| 156 | +``` |
| 157 | + |
| 158 | +### If the pattern matches against AST metadata, add a skip clause: |
| 159 | + |
| 160 | +If you find that the pattern is producing duplicate matches, it might be matching against metadata. To verify this, try parsing the code and inspecting the metadata: |
| 161 | + |
| 162 | +```wl |
| 163 | +In[1]:= CodeParser`CodeParse["x = 1"] |
| 164 | +
|
| 165 | +Out[1]= CodeParser`ContainerNode[String, {CodeParser`CallNode[CodeParser`LeafNode[Symbol, "Set", <||>], {CodeParser`LeafNode[Symbol, "x", <|CodeParser`Source -> {{1, 1}, {1, 2}}|>], CodeParser`LeafNode[Integer, "1", <|CodeParser`Source -> {{1, 5}, {1, 6}}|>]}, <|CodeParser`Source -> {{1, 1}, {1, 6}}, "Definitions" -> {CodeParser`LeafNode[Symbol, "x", <|CodeParser`Source -> {{1, 1}, {1, 2}}|>]}|>]}, <|CodeParser`Source -> {{1, 1}, {1, 6}}|>] |
| 166 | +``` |
| 167 | + |
| 168 | +Note that the ``CodeParser`LeafNode[Symbol, "x", <|...|>]`` appears twice: once in the `CallNode` tree and once in the `"Definitions"` metadata. |
| 169 | + |
| 170 | +To avoid this, you can add a skip clause to the handler to skip the match: |
| 171 | + |
| 172 | +```wl |
| 173 | +inspectMyNewRule[ pos_, ast_ ] /; MemberQ[ pos, _Key ] := { }; |
| 174 | +``` |
| 175 | + |
| 176 | +This works because only metadata will have `Key[...]` positions: |
| 177 | + |
| 178 | +```wl |
| 179 | +In[2]:= Position[CodeParser`CodeParse["x = 1"], CodeParser`LeafNode[Symbol, "x", _]] |
| 180 | +
|
| 181 | +Out[2]= {{2, 1, 2, 1}, {2, 1, 3, Key["Definitions"], 1}} |
| 182 | +``` |
| 183 | + |
| 184 | +### If extracting a string field (e.g., symbol name) for the message: |
| 185 | + |
| 186 | +```wl |
| 187 | +inspectMyNewRule[ pos_, ast_ ] := |
| 188 | + Enclose @ Module[ { node, name, as }, |
| 189 | + node = ConfirmMatch[ Extract[ ast, pos ], _[ _, _, __ ], "Node" ]; |
| 190 | + name = ConfirmBy[ node[[ 2 ]], StringQ, "Name" ]; |
| 191 | + as = ConfirmBy[ node[[ 3 ]], AssociationQ, "Metadata" ]; |
| 192 | + ci`InspectionObject[ |
| 193 | + "MyRuleTag", |
| 194 | + "The symbol ``" <> name <> "`` has an issue", |
| 195 | + "Warning", |
| 196 | + <| as, ConfidenceLevel -> 0.9 |> |
| 197 | + ] |
| 198 | + ]; |
| 199 | +``` |
| 200 | + |
| 201 | +### Text-level handler template: |
| 202 | + |
| 203 | +```wl |
| 204 | +inspectMyTextRule // beginDefinition; |
| 205 | +
|
| 206 | +inspectMyTextRule[ lines_List ] := MapIndexed[ |
| 207 | + Function[ { line, idx }, |
| 208 | + If[ someCondition @ line, |
| 209 | + ci`InspectionObject[ |
| 210 | + "MyTextRuleTag", |
| 211 | + "Description of the issue", |
| 212 | + "Formatting", |
| 213 | + <| cp`Source -> { { First @ idx, 1 }, { First @ idx, StringLength @ line } }, ConfidenceLevel -> 0.95 |> |
| 214 | + ], |
| 215 | + Nothing |
| 216 | + ] |
| 217 | + ], |
| 218 | + lines |
| 219 | +]; |
| 220 | +
|
| 221 | +inspectMyTextRule // endDefinition; |
| 222 | +``` |
| 223 | + |
| 224 | +### If the handler needs a test predicate, define it nearby: |
| 225 | + |
| 226 | +```wl |
| 227 | +myPredicateQ // beginDefinition; |
| 228 | +myPredicateQ[ name_String ] := (* condition *); |
| 229 | +myPredicateQ[ ___ ] := False; |
| 230 | +myPredicateQ // endDefinition; |
| 231 | +``` |
| 232 | + |
| 233 | +## Step 8: Write Tests |
| 234 | + |
| 235 | +Add tests to `Tests/CodeInspectorTool.wlt` at the end, before the cleanup section (`Integration Tests - Cleanup`). Follow this exact pattern: |
| 236 | + |
| 237 | +```wl |
| 238 | +(* ::**************************************************************************************************************:: *) |
| 239 | +(* ::Section::Closed:: *) |
| 240 | +(*Custom Rules - MyRuleTag*) |
| 241 | +
|
| 242 | +(* ::**************************************************************************************************************:: *) |
| 243 | +(* ::Subsection::Closed:: *) |
| 244 | +(*inspectMyNewRule - Basic Detection*) |
| 245 | +VerificationTest[ |
| 246 | + $myRuleResult = CodeInspectorToolFunction @ <| |
| 247 | + "code" -> "problematic code here", |
| 248 | + (* only include additional parameters if needed for the test *) |
| 249 | + |>, |
| 250 | + _String, |
| 251 | + SameTest -> MatchQ, |
| 252 | + TestID -> "MyRuleTag-Basic-ReturnsString" |
| 253 | +] |
| 254 | +
|
| 255 | +VerificationTest[ |
| 256 | + StringCount[ $myRuleResult, "Issue " ~~ DigitCharacter.. ~~ ": MyRuleTag" ], |
| 257 | + 1, |
| 258 | + SameTest -> SameQ, |
| 259 | + TestID -> "MyRuleTag-Basic-HasTag" |
| 260 | +] |
| 261 | +
|
| 262 | +VerificationTest[ |
| 263 | + StringContainsQ[ $myRuleResult, "Description of the issue" ], |
| 264 | + True, |
| 265 | + SameTest -> SameQ, |
| 266 | + TestID -> "MyRuleTag-Basic-HasDescription" |
| 267 | +] |
| 268 | +
|
| 269 | +VerificationTest[ |
| 270 | + StringContainsQ[ $myRuleResult, "(Warning" ], |
| 271 | + True, |
| 272 | + SameTest -> SameQ, |
| 273 | + TestID -> "MyRuleTag-Basic-HasSeverity" |
| 274 | +] |
| 275 | +``` |
| 276 | + |
| 277 | +**Always include:** |
| 278 | +- A detection test (returns `_String` result containing the expected number of occurrences of the tag) |
| 279 | +- A description test (message text appears) |
| 280 | +- A severity test (correct severity in output) |
| 281 | +- A false-positive test (clean code does NOT trigger the rule) |
| 282 | + |
| 283 | +## Step 9: Verify |
| 284 | + |
| 285 | +1. **Run the tests** using the `TestReport` MCP tool on `Tests/CodeInspectorTool.wlt` |
| 286 | +2. **Run CodeInspector** on the modified `Kernel/Tools/CodeInspector/Rules.wl` to check for issues |
| 287 | +3. Fix any failures and re-run until all tests pass |
| 288 | + |
| 289 | +## Reference: Key Aliases in Rules.wl |
| 290 | + |
| 291 | +These context aliases are used throughout the file: |
| 292 | + |
| 293 | +| Alias | Context | Aliased Example | Target Symbol | |
| 294 | +| --- | --- | --- | --- | |
| 295 | +| `ci` | `CodeInspector` | ``ci`InspectionObject`` | ``CodeInspector`InspectionObject`` | |
| 296 | +| `cp` | `CodeParser` | ``cp`CallNode`` | ``CodeParser`CallNode`` | |
| 297 | + |
| 298 | +## Reference: Section Marker Format |
| 299 | + |
| 300 | +Use this exact format for section/subsection markers in Rules.wl: |
| 301 | +``` |
| 302 | +(* ::**************************************************************************************************************:: *) |
| 303 | +(* ::Subsection::Closed:: *) |
| 304 | +(*functionName*) |
| 305 | +``` |
0 commit comments