Adding missing @Override annotations (part 3). Adding RRA/RWA/RUI annotations. Some refactoring by unv-unv · Pull Request #78 · consulo/consulo-groovy

unv-unv · 2026-07-24T20:19:14Z

No description provided.

…otations. Some refactoring.

unv-unv · 2026-07-24T20:19:46Z

@claude review pr with inline comments

github-actions · 2026-07-24T20:20:10Z

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

PR Review — #78 (Adding missing `@Override` / RRA/RWA/RUI annotations, part 3)

Gather context & read the full diff (50 files)
Analyze changes for correctness/quality
Post inline review comments

Summary

This is a large but overwhelmingly mechanical and safe change. The bulk of it is:

Adding missing @Override annotations (compiler-verified, so these are low-risk by construction).
Adding @RequiredReadAction / @RequiredWriteAction / @RequiredUIAccess contract annotations to PSI methods.
Modern-Java cleanups: diamond <>, pattern-matching instanceof, lambdas replacing anonymous Runnable/Computable/CachedValueProvider.
API modernization: hasModifierProperty(PsiModifier.STATIC/FINAL) → isStatic()/isFinal(), GroovyFileType.GROOVY_LANGUAGE → GroovyLanguage.INSTANCE, CompilerBundle → CompilerLocalize, ObjectUtil.assertNotNull → Objects.requireNonNull, and @date/User:/@autor Javadoc headers → @author/@since (ISO-8601).

I found no correctness/behavioral bugs — the runReadAction, lambda, and instanceof-pattern rewrites are all behavior-preserving. I left 2 inline suggestions, both minor consistency nits that fit this PR's own theme:

ExcludeFromStubGenerationAction.update(AnActionEvent) — got @RequiredReadAction but is missing the @Override that this PR is specifically about. Every other update(AnActionEvent) override in the repo has it. (inline suggestion)
GrInterfaceDefinitionImpl — @since 16.03.2007 kept the old dd.MM.yyyy format while the rest of the PR normalized these to ISO-8601 (@since 2007-03-16). (inline suggestion)

Minor observations (non-blocking, no change requested)

A few touched toArray(new T[collection.size()]) call sites (e.g. GrDocMethodParamsImpl.getParameterTypes) could use the toArray(EMPTY_ARRAY) idiom, but they're pre-existing and out of scope.
GrDynamicMethodImpl's constructor now carries @RequiredReadAction, which propagates a read-action requirement to all call sites — intended given it touches method.getManager(), just noting it.

Overall: LGTM once the two nits above are addressed (or consciously skipped). Nice cleanup.
· branch overrides-3

github-actions · 2026-07-24T20:22:43Z

+    );
  }

+  @RequiredReadAction


This PR is about adding missing @Override annotations, but update(AnActionEvent) overrides AnAction.update and only got @RequiredReadAction here — the @Override is missing. Every other update(AnActionEvent) override in this repo (e.g. RemoveDynamicAction, ExpandAllAction, MvcActionGroup) carries @Override, so this one is inconsistent.

Suggested change

@RequiredReadAction

@Override

@RequiredReadAction

github-actions · 2026-07-24T20:22:52Z

 /**
 * @author Dmitry.Krasilschikov
- * @date 16.03.2007
+ * @since 16.03.2007


Elsewhere in this PR the @date → @since conversions were also normalized to ISO-8601 (e.g. GrInterfaceDefinition now reads @since 2007-03-18, GrExtendsClauseImpl @since 2007-03-26). This one kept the old dd.MM.yyyy layout, so it's inconsistent.

Suggested change

* @since 16.03.2007

* @since 2007-03-16

unv-unv · 2026-07-24T22:54:36Z

@claude review pr with inline comments

github-actions · 2026-07-24T22:54:53Z

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

PR Review — #78 (inline comments)

Read the full diff (50 files)
Analyze for correctness/quality
Post inline review comments

Summary

Large but overwhelmingly mechanical and safe change. The bulk is: adding compiler-verified @Overrides; adding @RequiredReadAction / @RequiredWriteAction / @RequiredUIAccess contract annotations; and modern-Java cleanups (diamond <>, instanceof pattern binding, lambdas replacing anonymous Runnable/Computable/CachedValueProvider, hasModifierProperty(STATIC/FINAL) → isStatic()/isFinal(), GroovyFileType.GROOVY_LANGUAGE → GroovyLanguage.INSTANCE, CompilerBundle → CompilerLocalize, ObjectUtil.assertNotNull → Objects.requireNonNull, and @date/User:/@autor headers → @author/@since ISO-8601).

The runReadAction/lambda and instanceof-pattern rewrites are all behavior-preserving. CI (build-plugin) is green, so it all compiles — including the @RequiredReadAction-annotated local Runnable in GrFinalListener (legal, though the marker has no contract effect on a local var). I found no PR-introduced correctness/behavioral bugs.

Actionable (2 inline suggestions posted)

ExcludeFromStubGenerationAction.update(AnActionEvent) — got @RequiredReadAction but is missing the @Override this PR is specifically about; every other update(AnActionEvent) override in the repo has it. (inline)
GrInterfaceDefinitionImpl — @since 16.03.2007 kept the old dd.MM.yyyy format while the rest of the PR normalized these to ISO-8601 (@since 2007-03-16). (inline)

Non-blocking observations (out of scope, no change requested)

GrDocParameterReferenceImpl.multiResolve — the type-parameter else branch populates candidates but then falls through to return ResolveResult.EMPTY_ARRAY, so groovydoc @param <T> references never resolve. This is pre-existing (the PR only reworked the instanceof here), just flagging it as a latent bug.
A few touched toArray(new T[collection.size()]) sites (e.g. GrDocMethodParamsImpl.getParameterTypes) could use the toArray(EMPTY_ARRAY) idiom — pre-existing, out of scope.
GrDynamicMethodImpl's constructor now carries @RequiredReadAction, propagating a read-action requirement to all call sites — intended, since it touches method.getManager().

Overall: LGTM once the two nits above are addressed (or consciously skipped). Nice cleanup.
· branch overrides-3

github-actions · 2026-07-24T22:56:55Z

+    );
  }

+  @RequiredReadAction


This PR is specifically about adding missing @Override annotations, yet update(AnActionEvent) — which overrides AnAction.update — only received @RequiredReadAction; the @Override is missing. Every other update(AnActionEvent) override in this repo carries it, so this one is inconsistent.

Suggested change

@RequiredReadAction

@Override

@RequiredReadAction

github-actions · 2026-07-24T22:57:02Z

 /**
 * @author Dmitry.Krasilschikov
- * @date 16.03.2007
+ * @since 16.03.2007


Elsewhere in this PR the @date/@autor headers converted to @since were normalized to ISO-8601 (e.g. GrInterfaceDefinition → @since 2007-03-18, GrExtendsClauseImpl → @since 2007-03-26). This one kept the old dd.MM.yyyy layout, so it's inconsistent.

Suggested change

* @since 16.03.2007

* @since 2007-03-16

unv-unv · 2026-07-25T08:14:04Z

@claude review pr with inline comments

github-actions · 2026-07-25T08:14:47Z

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

PR Review — #78 (inline comments)

Read the full diff (60 files)
Analyze for correctness / quality
Post inline review comments

Summary

Still a large but overwhelmingly mechanical and safe change: compiler-verified @Overrides; @RequiredReadAction / @RequiredWriteAction / @RequiredUIAccess contract markers; and modern-Java cleanups (diamond <>, instanceof pattern binding, lambdas replacing anonymous Runnable/Computable/CachedValueProvider, hasModifierProperty(STATIC/FINAL) → isStatic()/isFinal(), GroovyFileType.GROOVY_LANGUAGE → GroovyLanguage.INSTANCE, CompilerBundle → CompilerLocalize, ObjectUtil.assertNotNull → Objects.requireNonNull, @date/@autor/User: headers → @author/@since ISO-8601). Several actions were also migrated from sync update to updateAsync + ActionSafeReadLock.

Both nits from my previous reviews are now resolved:

✅ ExcludeFromStubGenerationAction — update was replaced by updateAsync (which carries @Override), and actionPerformed now has @Override.
✅ GrInterfaceDefinitionImpl — @since is now 2007-03-16 (ISO-8601).

I verified the MvcConsole lambda rewrite compiles (the application/modalityState locals are declared at the top of executeProcessImpl), and the instanceof-pattern / runReadAction rewrites are all behavior-preserving. I found no PR-introduced correctness/behavioral bugs.

Actionable (2 inline suggestions posted)

ExcludeFromStubGenerationAction.updateAsync — binds the lambda param presentation but calls e.getPresentation() instead of using it. GroovyShellActionBase.updateAsync (same PR) uses the bound presentation; recommend matching for consistency. (inline)
GenerateGroovyDocAction.updateAsync — same nit: binds presentation, then calls e.getPresentation(). (inline)

Non-blocking observations (pre-existing, out of scope)

GrDocParameterReferenceImpl.multiResolve — both branches populate the local candidates list, but the method unconditionally return ResolveResult.EMPTY_ARRAY;, so groovydoc @param / @param <T> references never resolve. Pre-existing (the PR only reworked the instanceof binding); the buggy return line is untouched. Worth a follow-up, but fixing it is a behavior change beyond this PR's scope.
A few touched toArray(new T[collection.size()]) sites (e.g. GrDocMethodParamsImpl.getParameterTypes, GriffonFramework.getSourceFolders) could use the toArray(EMPTY_ARRAY) idiom — pre-existing, out of scope.
GrDynamicMethodImpl's constructor carries @RequiredReadAction (touches method.getManager()), propagating a read-action requirement to all call sites — intended.

Overall: LGTM. The two remaining items are minor consistency nits; feel free to apply or consciously skip. Nice cleanup.
· branch overrides-3

github-actions · 2026-07-25T08:17:01Z

-      return false;
+    @Override
+    public Coroutine<?, ?> updateAsync(AnActionEvent e) {
+        return ActionSafeReadLock.run(e, presentation -> e.getPresentation().setEnabledAndVisible(isEnabled(e))).toCoroutine();


This updateAsync binds the lambda parameter presentation but then calls e.getPresentation() instead of using it. GroovyShellActionBase (rewritten to updateAsync in this same PR) uses the bound presentation handle — this is the intended safe-read-lock presentation to mutate. Recommend using it here too for consistency:

Suggested change

return ActionSafeReadLock.run(e, presentation -> e.getPresentation().setEnabledAndVisible(isEnabled(e))).toCoroutine();

return ActionSafeReadLock.run(e, presentation -> presentation.setEnabledAndVisible(isEnabled(e))).toCoroutine();

github-actions · 2026-07-25T08:17:05Z

+        return ActionSafeReadLock.run(e, presentation -> {
+            Module module = e.getData(Module.KEY);
+
+            e.getPresentation().setEnabledAndVisible(module != null && LibrariesUtil.hasGroovySdk(module));


Same nit as ExcludeFromStubGenerationAction: the lambda binds presentation but the body calls e.getPresentation() instead. Use the bound presentation for consistency with GroovyShellActionBase.updateAsync in this PR:

Suggested change

e.getPresentation().setEnabledAndVisible(module != null && LibrariesUtil.hasGroovySdk(module));

presentation.setEnabledAndVisible(module != null && LibrariesUtil.hasGroovySdk(module));

unv-unv · 2026-07-25T08:26:10Z

@claude review pr with inline comments

github-actions · 2026-07-25T08:26:31Z

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

PR Review — #78 (inline comments)

Read the full diff (60 files)
Analyze for correctness / quality
Post inline review comments

Summary

Still a large but overwhelmingly mechanical and safe change: compiler-verified @Overrides; @RequiredReadAction / @RequiredWriteAction / @RequiredUIAccess contract markers; sync→updateAsync + ActionSafeReadLock action migrations; and modern-Java cleanups (diamond <>, instanceof pattern binding, lambdas replacing anonymous Runnable/Computable/CachedValueProvider, hasModifierProperty(STATIC/FINAL) → isStatic()/isFinal(), GroovyFileType.GROOVY_LANGUAGE → GroovyLanguage.INSTANCE, CompilerBundle → CompilerLocalize, ObjectUtil.assertNotNull → Objects.requireNonNull, @date/@autor/User: headers → @author/@since ISO-8601).

All items from previous reviews are now resolved:

✅ ExcludeFromStubGenerationAction.updateAsync / GenerateGroovyDocAction.updateAsync now use the bound presentation handle (matching GroovyShellActionBase), rather than e.getPresentation().
✅ GrInterfaceDefinitionImpl @since is ISO-8601 (2007-03-16).
✅ GrDocParameterReferenceImpl.multiResolve — the long-standing latent bug (both branches populated candidates but the method unconditionally returned EMPTY_ARRAY) is fixed: it now returns candidates.toArray(...), so groovydoc @param / @param <T> references resolve.

I verified the MvcConsole lambda rewrite compiles — application / modalityState are declared at the top of executeProcessImpl (lines 267-268), and the captured module/onDone locals are effectively final. The runReadAction / activate(...) lambda and all instanceof-pattern rewrites are behavior-preserving. I found no PR-introduced correctness/behavioral bugs.

Actionable (1 inline suggestion posted)

GrEnumConstantImpl.MyReference.bindToElement — annotated @RequiredReadAction, but every other bindToElement touched in this PR is @RequiredWriteAction (e.g. GrDocParameterReferenceImpl, GrDocMemberReferenceImpl), and it's the write-side counterpart of handleElementRename right above it. Minor consistency nit, matching this PR's own theme. (inline)

Non-blocking observations (out of scope)

A few touched toArray(new T[collection.size()]) sites (e.g. GrDocMethodParamsImpl.getParameterTypes, GriffonFramework.getSourceFolders) could use the toArray(EMPTY_ARRAY) idiom — pre-existing.
GrDynamicMethodImpl's constructor carries @RequiredReadAction (touches method.getManager()), propagating a read-action requirement to all call sites — intended.

Overall: LGTM. One remaining minor consistency nit; feel free to apply or consciously skip. Nice cleanup.
· branch overrides-3

github-actions · 2026-07-25T08:29:09Z

    }

    @Override
+    @RequiredReadAction


Consistency nit (fits this PR's RRA/RWA theme): this bindToElement got @RequiredReadAction, but every other bindToElement touched in this PR is annotated @RequiredWriteAction (e.g. GrDocParameterReferenceImpl, GrDocMemberReferenceImpl), and it's the write-side counterpart of handleElementRename just above (which correctly got @RequiredWriteAction). Recommend matching:

Suggested change

@RequiredReadAction

@RequiredWriteAction

unv-unv · 2026-07-25T08:40:02Z

@claude review pr with inline comments

github-actions · 2026-07-25T08:40:20Z

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

PR Review — #78 (inline comments)

Read the full diff (60 files)
Analyze for correctness / quality
Post inline review comments

Summary

Still a large but overwhelmingly mechanical and safe change: compiler-verified @Overrides; @RequiredReadAction / @RequiredWriteAction / @RequiredUIAccess contract markers; sync→updateAsync + ActionSafeReadLock action migrations; and modern-Java cleanups (diamond <>, instanceof pattern binding, lambdas replacing anonymous Runnable/Computable/CachedValueProvider, hasModifierProperty(STATIC/FINAL) → isStatic()/isFinal(), GroovyFileType.GROOVY_LANGUAGE → GroovyLanguage.INSTANCE, CompilerBundle → CompilerLocalize, ObjectUtil.assertNotNull → Objects.requireNonNull, @date/@autor/User: headers → @author/@since ISO-8601).

All items from previous reviews remain resolved (ExcludeFromStubGenerationAction/GenerateGroovyDocAction updateAsync use the bound presentation; GrInterfaceDefinitionImpl @since is ISO-8601; GrDocParameterReferenceImpl.multiResolve now returns candidates.toArray(...); GrEnumConstantImpl.MyReference.bindToElement is @RequiredWriteAction).

I re-verified the behavior-sensitive rewrites:

MvcConsole.executeProcessImpl — the lambda/invokeLater rewrite is behavior-preserving; the inner catch (Exception e1) was correctly renamed to avoid the shadow that the old nested anonymous class allowed, and the inner pic → thisPic rename avoids capturing the outer param.
getData(...) + null-check → getRequiredData(...) migrations (CollapseAllAction, ExpandAllAction, RemoveDynamicAction, GenerateGroovyDocAction, GroovyShellActionBase) are safe: each is either guarded by an updateAsync/update that disables the action, or only ever runs in a project/module context.
instanceof-pattern and runReadAction/activate(...) lambda rewrites are all equivalent.

I found no PR-introduced correctness/behavioral bugs. build-plugin CI is currently in progress; prior commits on this branch were green and this delta is mechanical.

Actionable (1 inline suggestion posted)

GroovyDocReducePackageAction.update — the collapsed one-line body is indented 6 spaces while the rest of the file uses 2-space indentation. Trivial formatting nit that fits this PR's consistency theme. (inline)

Non-blocking observations (out of scope)

A few touched toArray(new T[collection.size()]) sites (e.g. GrDocMethodParamsImpl.getParameterTypes, GriffonFramework.getSourceFolders/getTestFolders) could use the toArray(EMPTY_ARRAY) idiom — pre-existing.
GrDynamicMethodImpl's constructor carries @RequiredReadAction (touches method.getManager()), propagating a read-action requirement to all call sites — intended.
ExcludeFromStubGenerationAction was reindented to 4-space (matching GenerateGroovyDocAction), which differs from the 2-space convention used elsewhere in the module — cosmetic, not flagging.

Overall: LGTM. Only one trivial formatting nit remains; feel free to apply or skip. Nice cleanup.
· branch overrides-3

github-actions · 2026-07-25T08:43:06Z

-    } else {
-      presentation.setEnabled(true);
-    }
+      e.getPresentation().setEnabled(myPackagesList.getSelectedIndex() != -1);


Minor formatting nit: the update body is indented 6 spaces while the rest of this file uses 2-space indentation. Fits this PR's consistency theme:

Suggested change

e.getPresentation().setEnabled(myPackagesList.getSelectedIndex() != -1);

e.getPresentation().setEnabled(myPackagesList.getSelectedIndex() != -1);

Adding missing @OverRide annotations (part 3). Adding RRA/RWA/RUI ann…

69913b8

…otations. Some refactoring.

github-actions Bot reviewed Jul 24, 2026

View reviewed changes

Fixing actions.

1a2abb4

github-actions Bot reviewed Jul 25, 2026

View reviewed changes

unv-unv added 2 commits July 25, 2026 11:21

Fixes.

624c703

Fixing bug in GrDocParameterReferenceImpl.multiResolve.

f499036

github-actions Bot reviewed Jul 25, 2026

View reviewed changes

Marking GrEnumConstantImpl.MyReference.bindToElement with RWA.

cfb819d

github-actions Bot reviewed Jul 25, 2026

View reviewed changes

VISTALL merged commit 98536a2 into consulo:master Jul 25, 2026
1 check passed

unv-unv deleted the overrides-3 branch July 27, 2026 16:54

	return ActionSafeReadLock.run(e, presentation -> e.getPresentation().setEnabledAndVisible(isEnabled(e))).toCoroutine();
	return ActionSafeReadLock.run(e, presentation -> presentation.setEnabledAndVisible(isEnabled(e))).toCoroutine();

	e.getPresentation().setEnabledAndVisible(module != null && LibrariesUtil.hasGroovySdk(module));
	presentation.setEnabledAndVisible(module != null && LibrariesUtil.hasGroovySdk(module));

	e.getPresentation().setEnabled(myPackagesList.getSelectedIndex() != -1);
	e.getPresentation().setEnabled(myPackagesList.getSelectedIndex() != -1);

Uh oh!

Conversation

unv-unv commented Jul 24, 2026

Uh oh!

unv-unv commented Jul 24, 2026

Uh oh!

github-actions Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — #78 (Adding missing @Override / RRA/RWA/RUI annotations, part 3)

Summary

Minor observations (non-blocking, no change requested)

Uh oh!

github-actions Bot Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

unv-unv commented Jul 24, 2026

Uh oh!

github-actions Bot commented Jul 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — #78 (inline comments)

Summary

Actionable (2 inline suggestions posted)

Non-blocking observations (out of scope, no change requested)

Uh oh!

github-actions Bot Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 24, 2026

Choose a reason for hiding this comment

Uh oh!

unv-unv commented Jul 25, 2026

Uh oh!

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — #78 (inline comments)

Summary

Actionable (2 inline suggestions posted)

Non-blocking observations (pre-existing, out of scope)

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

unv-unv commented Jul 25, 2026

Uh oh!

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — #78 (inline comments)

Summary

Actionable (1 inline suggestion posted)

Non-blocking observations (out of scope)

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

unv-unv commented Jul 25, 2026

Uh oh!

github-actions Bot commented Jul 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review — #78 (inline comments)

Summary

Actionable (1 inline suggestion posted)

Non-blocking observations (out of scope)

Uh oh!

github-actions Bot Jul 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

github-actions Bot commented Jul 24, 2026 •

edited

Loading

PR Review — #78 (Adding missing `@Override` / RRA/RWA/RUI annotations, part 3)

github-actions Bot commented Jul 24, 2026 •

edited

Loading

github-actions Bot commented Jul 25, 2026 •

edited

Loading

github-actions Bot commented Jul 25, 2026 •

edited

Loading

github-actions Bot commented Jul 25, 2026 •

edited

Loading