fix: eliminate false positives in v1.51.0 codemods#93
Merged
Conversation
experimental-form-decorators-to-stable:
- Skip property accesses (member expressions) when renaming object keys.
Previously, `obj.EXPERIMENTAL_formDecorators` inside a pair value was
incorrectly renamed, turning `X ?? EXPERIMENTAL_X` into `X ?? X`.
migrate-policy-query-user:
- Scope object pattern processing to PolicyQueryUser-typed patterns only.
Previously, ANY `{ token }` destructuring in files importing from
@backstage/plugin-permission-node was modified, causing false positives
on unrelated patterns like `req.body` and type aliases.
- Clean up empty destructuring: `const {} = await expr` becomes `await expr`.
- Preserve variable declarations whose bindings are used elsewhere: add
TODO comment above instead of replacing the entire statement.
- Detect shorthand_property_identifier usages when checking if a declared
binding is referenced downstream.
schultzp2020
force-pushed
the
fix/codemod-false-positives
branch
from
1b17394 to
a3e3d35
Compare
The readme script now runs `yarn format` on README.md after generating it, so the committed output always matches what prettier expects. This avoids the CI freshness check failing when prettier table formatting differs from the generator output.
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.
What
Fixes false positives in two v1.51.0 codemods discovered by running the migration recipe against the Backstage monorepo with
--target packagesand--target plugins.Changes
@backstage/experimental-form-decorators-to-stableThe AST rule matched
property_identifiernodes at any depth inside apair, which incorrectly renamed property accesses liketemplate.spec.EXPERIMENTAL_formDecoratorswhen they appeared inside a pair's value. This turned backward-compat fallback patterns (formDecorators ?? EXPERIMENTAL_formDecorators) into the redundantformDecorators ?? formDecorators.Fix: Skip any
property_identifierwhose parent is amember_expression.@backstage/migrate-policy-query-userFalse positive scoping — The codemod processed ALL object patterns with
tokenfields in any file importing from@backstage/plugin-permission-node, not justPolicyQueryUser-typed patterns. This caused incorrect modifications to unrelated destructuring ({ token, context }fromreq.body) and type aliases (AutocompleteHandler).Fix: Added
isPolicyQueryUserObjectPattern()guard — only process object patterns that are directly typed asPolicyQueryUseror destructure a trackedPolicyQueryUserbinding.Empty destructuring — When all fields were removed,
const {} = await exprwas left behind.Fix: Detect empty
{}result and simplify toawait expr(preserving side-effects) or remove the statement entirely.Deleted declarations still referenced —
replaceStatementWithTodoreplaced entire statements referencing removed fields, even when those statements defined variables used elsewhere (e.g.,const secrets = { ..., backstageToken: token }), breaking downstream references.Fix:
declaresUsedBinding()checks if the statement's declared bindings are used elsewhere (includingshorthand_property_identifiernodes like{ secrets }). When found, a TODO comment is inserted above instead of replacing the statement.New test fixtures
experimental-form-decorators-to-stablenullish-coalescing-fallback??fallback is not renamedmigrate-policy-query-userno-op-unrelated-token{ token }patterns are not modifiedmigrate-policy-query-userempty-destructuring-awaitconst { token }: PolicyQueryUser = await expr→await exprmigrate-policy-query-usertoken-in-used-declaration