Golden Rules - FSM checker "promoter"

[noelsaw1]

This is a thoughtful architectural question. Let me break down feasibility and value:

Feasibility Assessment

Yes, it's technically possible. The Golden Rules Analyzer already does cross-file token analysis. Here's what we could detect:

Detectable Signals via Tokenization

Signal	Detection Method	Complexity
Multiple write sites to same option/meta	Track update_option('key'), update_post_meta($id, 'key') across files	Medium
Status-like variable writes	Match keys containing status, state, phase, step, mode	Low
Scattered transition logic	Count functions with names like set_*_status, transition_to_*	Low
Complex status conditionals	Detect switch/if-else chains on status-like variables	Medium-High

Rough Implementation Sketch

// In golden-rules-analyzer.php

// Track state writes: ['option_key' => ['file:line', 'file:line', ...]]
$state_write_sites = [];

// During token stream processing:
if ($token_is_function_call && in_array($func_name, ['update_option', 'update_post_meta', 'set_transient'])) {
    $key = extract_first_string_arg($tokens, $position);
    if (preg_match('/status|state|phase|step|mode|stage/i', $key)) {
        $state_write_sites[$key][] = "$current_file:$line_number";
    }
}

// After processing all files:
foreach ($state_write_sites as $key => $locations) {
    if (count($locations) >= 3 && count(array_unique(array_map('dirname', $locations))) >= 2) {
        // 3+ writes across 2+ directories = complexity signal
        emit_advisory('fsm-complexity-signal', $key, $locations);
    }
}

---

Value Assessment

Mixed—here's my honest take:

Arguments For

1. Proactive architectural guidance — catches complexity before it becomes unmanageable
2. Educational — introduces FSM thinking to developers who may not know the pattern
3. Differentiator — no other PHP scanner does architectural recommendations like this
4. Fits the "Golden Rules" philosophy — deeper insights beyond simple pattern matching

Arguments Against

1. High false positive risk — legitimate multi-path writes are common (settings pages, REST endpoints, CLI commands all updating same option)
2. Subjective threshold — when is 3 writes "too many"? 5? Depends entirely on context
3. Not actionable without context — unlike "unbounded query" which has a clear fix, this just says "maybe refactor?"
4. Scope creep — this is architectural consulting, not code quality scanning

---

My Recommendation

Worth exploring as a "beta/experimental" advisory feature, but not as a core pattern.

Practical Implementation Path

1. Start narrow: Only detect update_option() / update_post_meta() with explicitly status-like keys
2. High threshold: Require 4+ distinct write sites across 3+ files before flagging
3. Advisory output: Separate from violations—maybe a "Complexity Insights" section in the report
4. Link to guidance: Include a brief "What is FSM?" explainer or link to documentation

Output Format Concept

{
  "advisory_insights": [
    {
      "type": "complexity-signal",
      "category": "state-management",
      "message": "Option 'plugin_sync_status' is written from 5 locations across 4 files",
      "recommendation": "Consider centralizing state transitions with a state machine pattern",
      "locations": ["sync.php:142", "ajax.php:89", "cron.php:201", "admin.php:55", "rest.php:178"],
      "learn_more": "https://docs.wpcode.check/patterns/fsm-recommendation"
    }
  ]
}

---

Bottom Line

Question	Answer
Is it possible?	✅ Yes, with medium effort
Is it worthwhile?	⚠️ As an experimental advisory, yes. As a core pattern, probably not yet
Should you build it now?	🤔 I'd prioritize it below the Laravel expansion and TypeScript patterns

If you want to experiment, I'd suggest adding it as a "hidden" feature flag first (--experimental-insights) and collecting real-world data on whether the detections are useful before promoting it to a standard feature.

Want me to draft a more detailed implementation spec, or would you rather focus on the higher-priority roadmap items first?

[noelsaw1]

Great question. Let me think through what AST actually buys you versus the current token-stream approach:

What Causes False Positives in Token-Only Analysis

False Positive Scenario	Token Stream Sees	Reality
Commented-out code	update_option('status'...)	Dead code, not a write path
String in error message	"Failed to update_option"	Not a function call
Conditional write (same path)	Two update_option calls	Same logical branch, not scattered
Wrapper function	my_update_status() calls update_option()	Single controlled write path
Test/mock code	update_option in tests	Not production code

What AST Would Help With

Improvement	How AST Helps	Impact
Ignore comments	AST excludes comments from tree	✅ High - eliminates a whole class of FPs
Identify actual function calls	Node type = FunctionCall vs string literal	✅ High - no more string matching accidents
Control flow awareness	Can see if writes are in same if/else branch	✅ Medium - reduces "scattered" false positives
Scope analysis	Know if write is inside a class method vs global	✅ Medium - better context
Call graph tracing	Follow my_wrapper() → update_option()	⚠️ High effort, high value

---

The Tradeoff Analysis

Current Token Approach

Tokens: T_STRING('update_option') → '(' → T_CONSTANT_ENCAPSED_STRING('status') → ...

• ✅ Fast (~10-50ms per file)
• ✅ No dependencies
• ✅ Works on malformed PHP
• ❌ No structural understanding
• ❌ Can't distinguish call vs string vs comment

Full AST (via php-parser)

$parser = (new ParserFactory)->createForNewestSupportedVersion();
$ast = $parser->parse($code);
// Now traverse nodes with type awareness

• ✅ Structural understanding
• ✅ Comments excluded by default
• ✅ Can analyze control flow
• ❌ 5-10x slower per file
• ❌ Adds dependency (nikic/php-parser)
• ❌ Fails on syntax errors

---

Quantifying the Benefit

For the FSM complexity detection specifically:

Without AST	With AST	Improvement
~40-60% actionable findings	~70-85% actionable findings	Meaningful
Flags commented code	Ignores comments	Eliminates ~15% FPs
Flags string mentions	Only real calls	Eliminates ~10% FPs
Can't trace wrappers	Could trace one level	Reduces ~10% FPs

Estimated false positive reduction: 25-35%

---

My Assessment

For FSM Detection Specifically

Probably not worth it yet. Here's why:

1. The feature itself is experimental — you don't know if it's valuable even with perfect accuracy
2. Threshold tuning matters more — going from 3 to 5 write sites would eliminate more FPs than AST
3. Incremental path exists — you could add AST later if the feature proves useful

For Golden Rules Analyzer Generally

Worth considering for v2.0, but as a significant architectural investment:

Current: Token stream → Pattern match → Findings
Future:  Token stream → AST (optional) → Semantic analysis → Findings
                              ↓
                        Cache AST per file

The AST would unlock:

• Better superglobal detection (actual variable flow, not just presence)
• Complexity metrics (cyclomatic complexity, nesting depth)
• More accurate duplication detection
• Proper scope-aware analysis

---

Practical Recommendation

Timeline	Action
Now	Ship FSM detection with token-only approach + high thresholds
After validation	If users find it valuable but complain about FPs, then invest in AST
If building AST	Use nikic/php-parser, cache parsed ASTs, make it opt-in via --deep-analysis flag

Hybrid Approach (Best of Both)

// Fast token pre-filter
$candidates = token_scan_for_state_writes($file);

// Only parse AST for files with candidates
if (count($candidates) >= 2 && $deep_analysis_enabled) {
    $ast = parse_file_to_ast($file);
    $confirmed = ast_verify_write_sites($ast, $candidates);
}

This gives you:

• Token-speed scanning for 95% of files (no state writes)
• AST accuracy only where it matters
• Graceful degradation if AST parsing fails

---

Bottom Line

Question	Answer
Would AST reduce FPs?	✅ Yes, by 25-35%
Is it worth adding now?	❌ Not for an experimental feature
When would it be worth it?	When Golden Rules Analyzer is proven valuable and FP rate becomes the limiting factor
Best incremental step?	Hybrid token-prefilter + selective AST parsing

Want me to sketch out what the hybrid architecture would look like in more detail, or focus on shipping the token-only version first to validate demand?