Adding missing @Override annotations (part 4). Adding RRA/RWA/RUI annotations. Some localization and refactoring

[unv-unv]

No description provided.

[unv-unv]

@claude review pr with inline comments

[github-actions]

Claude finished @unv-unv's task in 3m 39s —— View job

---

PR Review — @OverRide annotations, RRA/RWA/RUI, localization & refactoring

I reviewed all 62 changed files against origin/master.

• Read repo guidelines
• Analyze the full diff
• Post inline review comment(s)
• Summarize

Overall

This is a large but overwhelmingly mechanical and safe change: adding missing @Override annotations, adding @RequiredReadAction / @RequiredWriteAction / @RequiredUIAccess markers, converting *Bundle.message(...) to LocalizeValue (*Localize), pattern-matching instanceof, diamond operators, lambda conversions, Ref → SimpleReference, and hasModifierProperty(STATIC/ABSTRACT) → isStatic()/isAbstract(). I spot-checked the non-trivial control-flow rewrites (e.g. GroovyBlock.getChildAttributes, GrModifierListImpl.checkModifierProperty brace-merge, GrListOrMapImpl map inference) and they preserve the original semantics. 👍

Findings

1. LocalizeValue concatenated as a String — GrIntroduceHandlerBase.java:364 (inline comment with suggestion posted)
Since getRefactoringName() now returns LocalizeValue, getRefactoringName() + "…" uses LocalizeValue.toString() rather than the resolved text, so the message shows the wrong value. Should be getRefactoringName().get() + " is not available in current scope" (also fixes a pre-existing missing space).

Minor / non-blocking observations (no change required)

• GrIntroduceHandlerBase.java:354 — @RequiredUIAccess is placed on a local variable (Consumer<Scope> callback). Annotating a local has no runtime effect and doesn't propagate the contract to the lambda body; consider dropping it (or annotate getContextAndInvoke / the functional type instead). Worth double-checking it even compiles if @RequiredUIAccess doesn't target LOCAL_VARIABLE.
• addAnnotation(...) contract mismatch — GrModifierListImpl.addAnnotation is marked @RequiredWriteAction while GrLightModifierList.addAnnotation is marked @RequiredReadAction. Both implement the same PsiModifierList method; the annotation intent should probably be consistent.
• @RequiredReadAction on a static final field — GrIntroduceHandlerBase.java GR_EXPRESSION_RENDERER field carries @RequiredReadAction. The access-requirement applies when the function is invoked, not when the field is initialized, so the marker on the field is effectively a no-op.

Nice cleanups overall — the Logger.getInstance("#fqn") → Logger.getInstance(Class) conversions and the anonymous-class → lambda simplifications read well.
· branch overrides-4

[github-actions]

getRefactoringName() now returns a LocalizeValue (it was a String before this PR), so this expression relies on LocalizeValue.toString() via string concatenation. That is not the resolved display text — the user would see the raw LocalizeValue representation instead of the refactoring name. There is also a missing space before is.

Suggested change

	RefactoringLocalize.cannotPerformRefactoringWithReason(getRefactoringName() + "is not available in current scope"),
	RefactoringLocalize.cannotPerformRefactoringWithReason(getRefactoringName().get() + " is not available in current scope"),

The repo already has a dedicated refactoring.is.not.supported.in.the.current.context.0 message with a {0} placeholder — a parameterized LocalizeValue would be cleaner than concatenation here.

[unv-unv]

@claude review pr with inline comments

[github-actions]

Claude finished @unv-unv's task in 2m 49s —— View job

---

PR Review — @OverRide, RRA/RWA/RUI, localization & refactoring

• Read repo guidelines (no CLAUDE.md/AGENTS.md present)
• Analyze the full diff vs origin/master (62 files)
• Verify non-trivial rewrites & localization changes
• Post inline review comment(s) with suggestions
• Summarize

Overall

Large but overwhelmingly mechanical and safe: missing @Override, @RequiredReadAction/@RequiredWriteAction/@RequiredUIAccess markers, *Bundle.message(...) → LocalizeValue, pattern-matching instanceof, diamond operators, anonymous-class → lambda, Ref → SimpleReference, hasModifierProperty(STATIC/ABSTRACT) → isStatic()/isAbstract(), and Logger.getInstance("#fqn") → Logger.getInstance(Class).

I re-verified the non-trivial control-flow rewrites and they preserve semantics:

• GrModifierListImpl.checkModifierProperty — the two nested ifs (GrMethod in a GrTypeDefinitionBody, then interface check) are correctly merged into one flattened condition; brace balance is preserved.
• GrListOrMapImpl.MyTypesCalculator — typeElement.getType() instanceof PsiArrayType arrayType returns the same array type as the original; map-inference path unchanged.
• GrModifierListImpl/GrLightModifierList — addAnnotation is now @RequiredWriteAction on both implementations, so the contract is consistent (this resolves a concern raised in the earlier review pass). 👍

Findings

1. LocalizeValue concatenated as a String — GrIntroduceHandlerBase.java:364 — inline suggestion posted (discussion).
getRefactoringName() now returns LocalizeValue, so getRefactoringName() + "is not available in current scope" stringifies via LocalizeValue.toString() (raw representation, not resolved text) and also drops a space before is. Fix: getRefactoringName().get() + " is not available in current scope" — consistent with the plain-String overload already used at line 546. Fix this →

Minor / non-blocking (no change required)

• GrIntroduceHandlerBase.java:353-354 — @RequiredUIAccess is placed on a local variable (Consumer<Scope> callback). Annotating a local has no runtime effect and doesn't propagate the contract into the lambda body; consider dropping it (the enclosing getContextAndInvoke is already @RequiredUIAccess).
• GrIntroduceHandlerBase.java:93-94 — @RequiredReadAction on the static final GR_EXPRESSION_RENDERER field is effectively a no-op; the requirement applies when the function is invoked, not at field init.

Nice cleanups overall — the lambda/diamond conversions and Logger.getInstance(Class) migrations read well, and the localization migration is applied consistently.
· branch overrides-4

[github-actions]

getRefactoringName() now returns a LocalizeValue (it was a String before this PR), so getRefactoringName() + "is not available in current scope" invokes LocalizeValue.toString() on it — that is not the resolved display text, so the error hint would show the raw LocalizeValue representation instead of the refactoring name. There is also a missing space before is.

Since cannotPerformRefactoringWithReason(...) already accepts a plain String (see line 546, which passes StringUtil.notNullize(e.getMessage())), resolve the name with .get() and drop the redundant localizeTODO(...) wrapper:

Suggested change

	RefactoringLocalize.cannotPerformRefactoringWithReason(LocalizeValue.localizeTODO(getRefactoringName() + "is not available in current scope")),
	RefactoringLocalize.cannotPerformRefactoringWithReason(getRefactoringName().get() + " is not available in current scope"),

The repo also has a dedicated parameterized message (refactoring.is.not.supported.in.the.current.context.0 with a {0} placeholder) — using it would be cleaner than string concatenation, but the fix above is the minimal correctness change.