fix(linter): match LINT-KEYWORD on identifier tokens, not raw substrings (#707)

History/dev-diary/2026/2026-08-14-issue-707-lint-keyword-on-tokens.md

@@ -0,0 +1,127 @@
+# 2026-08-14 — LINT-KEYWORD was a substring search, not a lint (#707)
+
+## Symptom
+
+```wfl
+store s as "MNOP"
+display s
+```
+
+```text
+warning[LINT-KEYWORD]: Keyword 'NO' should be lowercase
+ = Change to 'no'
+```
+
+The control confirms the mechanism: `"MXYP"` reports nothing. The rule matched
+the substring `NO` inside `MNOP`, inside a string literal.
+
+Comments went the same way — `// Note:` produced `Keyword 'No'`, `// TODO:`
+produced `Keyword 'TO'` — as did ordinary words: `"Ineligible"` produced
+`Keyword 'In'`.
+
+Found while porting a PHP minifier, where the program needs the character-class
+constant
+`"abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_"`. That string
+contains `MNOP` and cannot be changed — it *is* the definition of `\w`. Combined
+with the companion `LINT-INDENT` defect (#706), it meant `--lint` could not be
+used as a gate at all.
+
+## Root cause
+
+`KeywordCasingRule::apply` took the parsed `Program` and ignored it
+(`_program`), then ran `source.find()` over the raw file for each of 26
+keywords, in `UPPERCASE` and `Mixedcase` forms. No tokenization, no word
+boundaries, no exclusion of strings or comments.
+
+The `Mixedcase` half is what made it collide with English: `No`, `To`, `In`,
+`For`, `Each`, `End`, `Check`, `If`, `Count`, `From` are ordinary words and
+common prefixes, so any file with prose comments was likely to trip several.
+
+It was also a **false negative**. `source.find` returns only the first hit, so
+the rule emitted at most one diagnostic per keyword per casing — 52 for a file
+of any size. A file with fifty genuine `STORE` keywords reported one. The rule
+could neither avoid reporting non-keywords nor finish reporting real ones.
+
+## The fact that shaped the fix
+
+WFL keywords are **case-sensitive**: `src/lexer/token.rs:17` is
+`#[token("store")]` with no `ignore(case)`. So `STORE` and `Store` never lex as
+keywords — they lex as *identifiers*. `STORE s AS "x"` fails with
+`Variable 'STORE s AS' is not defined`.
+
+That rules out the obvious repair. "Only report where the lexer produced a
+keyword token" would have reported **nothing**, silently deleting the lint while
+looking like a fix.
+
+The rule now flags **identifier tokens whose lowercased text is a keyword**,
+which is what it was always reaching for. All four defects fall out at once:
+string literals lex as string tokens, comments never reach the token stream,
+`Ineligible` lowercases to a non-keyword, and iterating tokens reports every
+occurrence rather than the first.
+
+Keyword-ness is decided by lexing the lowercased word and checking it yields a
+single non-identifier token — so the rule tracks the lexer instead of a
+hardcoded list that drifts. The 26-keyword array is gone.
+
+## The mechanic that was load-bearing
+
+The lexer merges adjacent identifier words into one multi-word `Identifier`
+token: `STORE counter` lexes as `Identifier("STORE counter")`. Checking the
+whole token text would have found no keyword — and silently deleted the lint
+again, in a second way.
+
+So the rule slices `byte_start..byte_end` out of the source and checks each
+whitespace-separated word, offsetting the column accordingly. A multi-word
+identifier cannot span lines (a newline flushes it), so the line is the token's
+and the column is exact.
+
+## Red evidence
+
+5 of 10 linter tests failed against the old rule. The incidental one is the
+nicest: the three-occurrence case returned two diagnostics — `STORE` once, plus
+a bogus `TO` matched *inside the word* `STORE`.
+
+```text
+---- test_keyword_casing_ignores_string_literals stdout ----
+string literal contents must not be linted, got [... message: "Keyword 'NO' should be lowercase" ...]
+```
+
+Red: `74114dc`. Green: `f6ff1f1`. 10/10 after.
+
+## Known limitation, unchanged by this work
+
+`--lint` runs after parsing, so a mis-cased keyword that *breaks* parsing never
+reaches the linter — `STORE alpha as 1` exits 2 with a parse error. LINT-KEYWORD
+therefore only fires on mis-cased words that still parse, e.g. `store Count as 5`.
+That is pre-existing CLI behavior, identical before and after, and is recorded
+here so the rule's real reach is not overstated.
+
+## Review round: the one token that is case-insensitive
+
+Codex and Devin both caught, independently, that the first cut of this rewrite
+lost coverage the old rule had.
+
+`yes`/`no`/`true`/`false` are the single case-*insensitive* production in the
+lexer:
+
+```rust
+#[regex("(?i:yes|no|true|false)", ...)]
+BooleanLiteral(bool),
+```
+
+So `store flag as YES` lexes as a `BooleanLiteral`, never as an `Identifier`,
+and an identifier-only rule skipped it silently. The pre-#707 implementation
+carried `yes` and `no` in its 26-keyword array and *did* warn on them — so the
+claim that "output for genuine hits is identical" was false for exactly those
+two keywords, in exactly the way a substring search happened to get right.
+
+The rule now checks `BooleanLiteral` spans as well. Everything else in the
+lexer is case-sensitive — `NothingLiteral` is `#[token("nothing")]`, so
+`Nothing` still arrives as an identifier and was always covered.
+
+Worth recording as a general shape: replacing a crude mechanism with a precise
+one silently drops whatever the crude mechanism covered by accident. The
+regression was invisible in the six tests written for the reported defect,
+because those tests were all about false *positives*.
+
+Red: `90bfb69`. Green: the boolean-literal branch, 12/12 after.

src/linter/mod.rs

@@ -1,5 +1,8 @@
 use crate::diagnostics::{DiagnosticReporter, Severity, WflDiagnostic};
+use crate::lexer::lex_wfl_with_positions;
+use crate::lexer::token::Token;
 use crate::parser::ast::{Program, Statement};
+use std::collections::HashMap;
 use std::path::Path;
 
 pub trait LintRule {
@@ -245,76 +248,63 @@ impl LintRule for KeywordCasingRule {
     ) -> Vec<WflDiagnostic> {
         let mut diagnostics = Vec::new();
 
-        if let Ok(file) = reporter.files.get(file_id) {
-            let source = file.source();
+        let Ok(file) = reporter.files.get(file_id) else {
+            return diagnostics;
+        };
+        let source = file.source();
+
+        // WFL keywords are case-sensitive, so `STORE` never lexes as a keyword —
+        // it lexes as an identifier. A mis-cased keyword is therefore an
+        // identifier token whose lowercase form *is* a keyword. Working from the
+        // token stream (instead of raw substring search over the source) keeps
+        // string literals and comments out of the rule entirely, never matches
+        // inside a longer word, and reports every occurrence rather than the first.
+        let mut keyword_cache: HashMap<String, bool> = HashMap::new();
+
+        for token in lex_wfl_with_positions(source) {
+            // The lexer merges adjacent identifier words into one multi-word
+            // identifier, so check each word separately. Words are ASCII and the
+            // separators are spaces/tabs, so the byte offset within the token's
+            // span is also the column offset. Multi-word identifiers never span
+            // lines (a newline flushes them), so the line is the token's line.
+            let text = match token.token {
+                Token::Identifier(ref name) => source
+                    .get(token.byte_start..token.byte_end)
+                    .unwrap_or(name.as_str()),
+                // `yes`/`no`/`true`/`false` are the one case-*insensitive*
+                // production in the lexer (`src/lexer/token.rs:446`), so `YES`
+                // lexes as a boolean literal and never reaches the identifier
+                // branch. The pre-#707 rule carried `yes`/`no` in its keyword
+                // array and warned on them, so they are checked here to keep
+                // that coverage rather than silently dropping it.
+                Token::BooleanLiteral(_) => {
+                    let Some(text) = source.get(token.byte_start..token.byte_end) else {
+                        continue;
+                    };
+                    text
+                }
+                _ => continue,
+            };
 
-            let keywords = [
-                "store",
-                "as",
-                "create",
-                "change",
-                "to",
-                "define",
-                "action",
-                "called",
-                "give",
-                "back",
-                "check",
-                "if",
-                "otherwise",
-                "end",
-                "count",
-                "from",
-                "for",
-                "each",
-                "in",
-                "while",
-                "display",
-                "yes",
-                "no",
-                "nothing",
-                "missing",
-                "undefined",
-            ];
-
-            for keyword in keywords.iter() {
-                let uppercase_keyword = keyword.to_uppercase();
-                let mixed_case_keyword = keyword
-                    .chars()
-                    .enumerate()
-                    .map(|(i, c)| {
-                        if i == 0 {
-                            c.to_uppercase().next().unwrap()
-                        } else {
-                            c
-                        }
-                    })
-                    .collect::<String>();
-
-                if let Some(pos) = source.find(&uppercase_keyword) {
-                    let line_col = line_col_from_pos(source, pos);
-                    diagnostics.push(WflDiagnostic::new(
-                        Severity::Warning,
-                        format!("Keyword '{uppercase_keyword}' should be lowercase"),
-                        Some(format!("Change to '{keyword}'")),
-                        "LINT-KEYWORD".to_string(),
-                        file_id,
-                        line_col.0,
-                        line_col.1,
-                        None,
-                    ));
+            for (offset, word) in word_positions(text) {
+                if !word.chars().any(char::is_uppercase) {
+                    continue;
                 }
 
-                if let Some(pos) = source.find(&mixed_case_keyword) {
-                    let line_col = line_col_from_pos(source, pos);
+                let lowercase = word.to_lowercase();
+                let is_keyword = *keyword_cache
+                    .entry(lowercase.clone())
+                    .or_insert_with(|| is_keyword(&lowercase));
+
+                if is_keyword {
                     diagnostics.push(WflDiagnostic::new(
                         Severity::Warning,
-                        format!("Keyword '{mixed_case_keyword}' should be lowercase"),
-                        Some(format!("Change to '{keyword}'")),
+                        format!("Keyword '{word}' should be lowercase"),
+                        Some(format!("Change to '{lowercase}'")),
                         "LINT-KEYWORD".to_string(),
                         file_id,
-                        line_col.0,
-                        line_col.1,
+                        token.line,
+                        token.column + offset,
                         None,
                     ));
                 }
@@ -325,6 +315,39 @@ impl LintRule for KeywordCasingRule {
     }
 }
 
+/// Split `text` into whitespace-separated words, paired with each word's byte
+/// offset within `text`.
+fn word_positions(text: &str) -> Vec<(usize, &str)> {
+    let mut words = Vec::new();
+    let mut start: Option<usize> = None;
+
+    for (index, ch) in text.char_indices() {
+        if ch.is_whitespace() {
+            if let Some(word_start) = start.take() {
+                words.push((word_start, &text[word_start..index]));
+            }
+        } else if start.is_none() {
+            start = Some(index);
+        }
+    }
+
+    if let Some(word_start) = start {
+        words.push((word_start, &text[word_start..]));
+    }
+
+    words
+}
+
+/// Whether `lowercase` (already lowercased) is a WFL keyword.
+///
+/// Determined by lexing it: a keyword lexes to exactly one non-identifier token.
+/// Asking the lexer keeps this rule in sync with the language automatically,
+/// instead of a hardcoded keyword list that drifts.
+fn is_keyword(lowercase: &str) -> bool {
+    let tokens = lex_wfl_with_positions(lowercase);
+    matches!(tokens.as_slice(), [only] if !matches!(only.token, Token::Identifier(_)))
+}
+
 struct TrailingWhitespaceRule;
 
 impl LintRule for TrailingWhitespaceRule {
@@ -532,26 +555,6 @@ fn to_snake_case(s: &str) -> String {
     result
 }
 
-fn line_col_from_pos(source: &str, pos: usize) -> (usize, usize) {
-    let mut line = 1;
-    let mut col = 1;
-
-    for (i, c) in source.char_indices() {
-        if i >= pos {
-            break;
-        }
-
-        if c == '\n' {
-            line += 1;
-            col = 1;
-        } else {
-            col += 1;
-        }
-    }
-
-    (line, col)
-}
-
 #[cfg(test)]
 mod tests;