fix: recognize embedded-argument keywords as used (#34)#35
Open
EAnathos wants to merge 3 commits into
Open
Conversation
Keywords defined with embedded arguments (e.g. `Click On ${element}`) were
never counted as used, because usage detection compared keyword names by exact
string equality. A call such as `Click On ${something}` (or with a literal
value, `Click On Button`) therefore did not match the definition and the
keyword was wrongly reported as unused.
Robot Framework resolves embedded arguments statically by treating them as
wildcards; this mirrors that behavior:
- Add `KeywordProperties.matches()` which resolves both exact and
embedded-argument keyword names, plus a shared `normalize_keyword_name()`
helper (now reused by `KeywordRegistry`).
- Use `matches()` in `KeywordRegistry.resolve()` (as a fallback after exact
matches) and in the usage counters of `KeywordUsageService`, replacing the
literal `list.count()` lookups.
Adds unit tests for `matches()` and regression tests for the usage counters.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…view)
Counting usages via KeywordProperties.matches() ignored Robot Framework's
resolution precedence: a call to an exact keyword (e.g. `Open Browser`) was also
counted for any embedded keyword whose pattern happened to match it (e.g.
`Open ${page}`), so a genuinely unused embedded keyword could be reported as
used and metrics were skewed.
Count a call for a keyword only when it actually resolves to it, using the
registry (exact match wins over embedded) as the single source of truth. A cheap
matches() pre-filter keeps the registry-wide resolve off the hot path.
Also simplifies the matching helpers (single loop over the prefix variants, no
redundant regex anchors now that callers use fullmatch).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…iadee#34 review) The exact-wins refactor routed usage counting through resolve(), which returns a single keyword. That also arbitrated exact-vs-exact collisions: when the same keyword name is defined in several resources and called unqualified, resolve() picks the first-registered one, so the others were counted as 0 usages and wrongly reported as unused ("candidate for removal"). Only embedded matches need the resolve() precedence gate. An exact name reference is a genuine reference to every keyword sharing that name, so count it unconditionally; keep the resolve() gate for embedded-argument matches only. Adds a regression test for a duplicately-named keyword called unprefixed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #34.
Keywords defined with embedded (parameterized) arguments — e.g.
Click On ${element}— were never counted as used. Usage detection compared keyword names by exact string equality, so a call likeClick On ${something}(or with a literal value,Click On Button) did not match the definition, and the keyword was wrongly reported as unused (also skewing the reusage rate).Changes
schemas/domain/keywords.pynormalize_keyword_name()(single source of truth for the existing lowercase / strip-spaces / strip-underscores normalization).KeywordProperties.matches(called_name)which resolves both exact and embedded-argument keyword names by building a regex where each${...}becomes a wildcard.registries/keyword_registry.pyresolve()now falls back tomatches()for embedded keywords (exact matches still take precedence)._normalize_keyword_name()delegates to the shared helper.services/keyword_usage_service.pykeyword.matches(called)instead of literallist.count()lookups.Tests
KeywordProperties.matches()(exact, embedded with differing variable name, literal value, prefix variants, and non-matches).Full backend suite: no new failures versus
main(the pre-existing failures are unrelated, caused by optional export dependencies not installed locally).Manual verification
Reproducer project (
Click On ${element}defined, called asClick On ${something}):roboview analyze→ Unused Keywords