`smells::string_error` false-positive on `Result<HashMap<String, String>>` — detector flags the String in the OK type as if it were the error type

[Vuk97]

desloppify version: 0.9.15
Detector: smells (sub-pattern string_error)

Bug

The smells detector pattern Result with String error type (id string_error) is supposed to flag functions like fn foo() -> Result<T, String>. But it actually flags any Result<...String...> regardless of which generic position the String appears in. It does not parse the generics — it just looks for the substring.

Steps to reproduce

desloppify detect smells --json

Actual output (verbatim)

$ desloppify detect smells --json
{
  "entries": [
    {
      "id": "string_error",
      "label": "Result with String error type",
      "severity": "medium",
      "count": 1,
      "files": 1,
      "matches": [
        {
          "file": "src/order.rs",
          "line": 64,
          "content": ") -> Result<std::collections::HashMap<String, String>> {"
        }
      ]
    },
    ...

Line 64 of src/order.rs is:

pub fn build_l2_headers(...) -> Result<std::collections::HashMap<String, String>> {

This is Result<HashMap<String, String>> — the Result is the standard anyhow::Result<T> (single generic, infers Error to anyhow::Error), and the Strings are the HashMap key and value types, not the Result's error type. There is no String in the error position anywhere in this signature.

Expected behavior

The detector should parse the Result<...> generic args and only flag when String appears in the second generic position (the explicit error type). Or, simpler, only flag the literal pattern Result<.+, String> (with a comma before String>).

Suggested fix

The current pattern is probably a regex like r'Result<[^>]*String[^>]*>'. Replace with:

# Match Result<T, String> only — String must follow a comma
STRING_ERROR_RE = re.compile(r'Result<[^,<>]+(?:<[^>]+>)*\s*,\s*String\s*>')

Or, more rigorously, use tree-sitter to parse the generic arguments and check that the second positional arg is String.

Impact

Triggers on legitimate code that returns HashMap<String, String> or any other type involving String inside the OK type. The detector flags this as a medium severity smell, suggesting the user "use a typed error enum instead" — but there's nothing to fix. In this codebase it surfaces once on build_l2_headers (returns the L2 auth headers as a HashMap<String, String>); a real codebase would hit it many more times on JSON serialization helpers, env-var loaders, etc.

Discovered while

Running desloppify detect smells --json to look for code-quality patterns. The false positive is a tier medium smell that contributes to the Code quality dimension drag.