Skip to content

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

Open
unv-unv wants to merge 1 commit into
consulo:masterfrom
unv-unv:overrides-2
Open

Adding missing @Override annotations (part 2). Adding RRA/RWA/RUI annotations. Some localization and refactoring#47
unv-unv wants to merge 1 commit into
consulo:masterfrom
unv-unv:overrides-2

Conversation

@unv-unv

@unv-unv unv-unv commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

No description provided.

…otations. Some localization and refactoring.
@unv-unv

unv-unv commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

@claude review pr with inline comments

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

Claude finished @unv-unv's task in 4m 5s —— View job


PR Review — @OverRide / RRA·RWA·RUI annotations + refactoring

  • Read repo guidelines
  • Analyze the diff (57 files)
  • Post inline review comments with suggestions
  • Summarize findings

I reviewed all 57 changed files against origin/master. This is a large but overwhelmingly mechanical, low-risk change: added @Override, @RequiredReadAction/@RequiredWriteAction/@RequiredUIAccess annotations, instanceof pattern matching, diamond <>, switch expressions, RefSimpleReference, Processor/Conditionjava.util.function.Predicate, and PyBundlePyLocalize. I did not find functional regressions.

Inline comments

  • 🧵 Mixed tab/space indentation (PyCallExpressionImpl — representative; also PyAssignmentStatementImpl, PyClassElementType, PyDotNetClassType, PyBinaryExpressionImpl, VariantsProcessor, …). New annotation lines are indented with 4 spaces inside otherwise tab-indented files. Suggestion block attached.
  • 🧵 PyType.getName() contract change — adding @RequiredReadAction to an interface method tightens the contract for all callers; correct, but a behavioral change worth confirming.

Other observations (non-blocking)

  • Predicate return value ignored in visitMembersPyJavaClassType/PyDotNetClassType call processor.test(...) and discard the result. This mirrors the old processor.process(...) behavior (pre-existing), so no regression, but the early-stop semantics of a predicate/processor are silently dropped.
  • IntroduceHandler myDialogTitleLocalizeValue — passed directly into CommonRefactoringUtil.showErrorHint(...) and .get() into PyIntroduceDialog. This relies on a LocalizeValue overload of showErrorHint; fine if it compiles (see build note below).
  • Collections2.filter/transform (Guava) → ContainerUtil in PyCustomType/MultiProcessDebugger — Guava returned lazy views, ContainerUtil returns eager copies. Both are only consumed by addAll/unmodifiableCollection, so behavior is equivalent.
  • MultiProcessDebugger.waitForConnect — the empty try { … } finally {} was correctly removed; behavior unchanged.
  • Minor: several toArray(new Object[list.size()]) calls remain (e.g. PyCustomType:220) that could be simplified to toArray(), but they're outside the changed lines.

Note on verification

I reviewed by reading the diff only — I did not run a Maven build or tests in this environment, so I can't independently confirm that the ProcessorPredicate swaps (ContainerUtil.process/filter, showErrorHint(LocalizeValue), etc.) all resolve to existing overloads. If CI is green, those are settled.

Overall: looks good to merge. The only thing I'd genuinely act on is the tab/space indentation so the annotation blocks stay consistent with each file's existing style.
· branch overrides-2

Comment on lines 48 to 51
@Nullable
@Override
@RequiredReadAction
public PyExpression getCallee()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed indentation (tabs + spaces). This file is tab-indented, but the newly added @Override / @RequiredReadAction lines use 4 spaces, so the annotation stack no longer lines up with the public signature below it. The same tabs-vs-spaces mix was introduced across many of the touched tab-indented files (e.g. PyAssignmentStatementImpl, PyClassElementType, PyDotNetClassType, PyBinaryExpressionImpl, VariantsProcessor). A reformat pass so annotations match the surrounding indentation would keep the diff clean.

Suggested change
@Nullable
@Override
@RequiredReadAction
public PyExpression getCallee()
@Nullable
@Override
@RequiredReadAction
public PyExpression getCallee()

Comment on lines 48 to 51
@Nullable
@Override
@RequiredReadAction
public PyExpression getCallee()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mixed indentation (tabs + spaces). This file is tab-indented, but the newly added @Override / @RequiredReadAction lines use 4 spaces, so the annotation stack no longer lines up with the public signature below it. The same tabs-vs-spaces mix was introduced across many of the touched tab-indented files (e.g. PyAssignmentStatementImpl, PyClassElementType, PyDotNetClassType, PyBinaryExpressionImpl, VariantsProcessor). A reformat pass so annotations match the surrounding indentation would keep the diff clean.

Suggested change
@Nullable
@Override
@RequiredReadAction
public PyExpression getCallee()
@Nullable
@Override
@RequiredReadAction
public PyExpression getCallee()

* @return name of the type
*/
@Nullable
@RequiredReadAction

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding @RequiredReadAction to this interface method tightens the contract for every implementor and caller of PyType.getName() — any call site not already inside a read action will now trip the assertion. That is almost certainly correct here (name resolution touches PSI), just flagging that this is a behavioral/API change rather than a pure annotation cleanup, so it is worth confirming the callers are all under a read lock.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant