Refactoring JavaKeywordCompletion and PsiJavaPatterns by unv-unv · Pull Request #310 · consulo/consulo-java

unv-unv · 2026-10-08T19:10:54Z

No description provided.

unv-unv · 2026-10-08T19:12:47Z

@claude review pr with inline comments

github-actions · 2026-10-08T19:13:09Z

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

Claude finished reviewing PR #310

Gather context and read the diff
Review PsiJavaPatterns.java
Review JavaKeywordCompletion.java
Post inline comments (review with 16 inline comments, each with a one-click suggestion)
Final summary

Summary

I found no logic changes. Most of the PR is reformatting (2 → 4 space indent, braces on single-line ifs), plus instanceof pattern-matching rewrites and new @RequiredReadAction annotations. I compared every non-whitespace change against the original and all of them behave the same: createTypeDeclaration, nextIsIdentifier, isInstanceofPlace, isVariableTypePosition, isInsideQualifierClass / isInsideInheritorClass, and the Conditions.instanceOf → PsiTypeElement.class::isInstance swap.

Inline comments

PsiJavaPatterns.java

The @RequiredReadAction annotations are on the wrong conditions. They were added to the accepts in psiJavaElement(Class...) and psiModifierListOwner, which only do instanceof checks. The conditions that actually read PSI have none: psiNewExpression (getClassOrAnonymousClassReference / getQualifiedName), and psiLiteral / literalExpression (getValue()).
Leftover final on parameters at L61, L84, L98 and L117, which doesn't match the rest of the cleanup.

JavaKeywordCompletion.java

Some methods that read PSI still lack @RequiredReadAction: isLambdaParameterType, getFileForDeclaration, expectsClassLiteral, isForLoopMachinery and superConstructorHasParameters.
Indentation slips from the reformat: in isSuitableForClass (L863–869) and addPrimitiveTypes (L946–950), the method-call arguments sit at the same level as the || operand.
addUnfinishedMethodTypeParameters: the pattern variable is named capture, which doesn't say what it matches. I suggested a rename; the pattern could also become a static final constant.
Smaller nits: a leftover final boolean hasParams (L499), and in isInsideInheritorClass a typed for loop would remove the (PsiClass) parent cast.

I didn't build or run tests; this is a read-only review.

github-actions

Inline review from Claude. Overall the refactoring looks behavior-preserving: I checked every non-whitespace hunk (pattern-matching instanceof rewrites, the flattened else if in createTypeDeclaration, the inlined boolean chains in nextIsIdentifier, isInstanceofPlace, isVariableTypePosition, isInsideQualifierClass and isInsideInheritorClass, and Conditions.instanceOf → PsiTypeElement.class::isInstance) and found no logic changes. The comments below are about where @RequiredReadAction is applied, leftover final modifiers, and a couple of indentation slips from the reformat.

github-actions · 2026-10-08T19:16:28Z

+            @Override
+            @RequiredReadAction


This accepts only calls Class.isInstance, so it never touches PSI and doesn't need @RequiredReadAction. Meanwhile the conditions that do read PSI (psiNewExpression, psiLiteral, literalExpression) have no annotation. I'd drop it here (and in psiModifierListOwner below) and add it where it actually applies.

Suggested change

@Override

@RequiredReadAction

@Override

github-actions · 2026-10-08T19:16:28Z

+            @Override
+            @RequiredReadAction


Same here: a plain instanceof check doesn't need a read action.

Suggested change

@Override

@RequiredReadAction

@Override

github-actions · 2026-10-08T19:16:28Z

+
+    public static PsiJavaElementPattern.Capture<PsiNewExpression> psiNewExpression(final String... fqns) {
+        return new PsiJavaElementPattern.Capture<>(new InitialPatternCondition<>(PsiNewExpression.class) {
+            @Override


This is the condition that needs @RequiredReadAction: getClassOrAnonymousClassReference() and getQualifiedName() both walk or resolve PSI. The same goes for PsiLiteral.getValue() in psiLiteral (L86) and literalExpression (L119).

Suggested change

@Override

@Override

@RequiredReadAction

github-actions · 2026-10-08T19:16:28Z

+    }
+
+    @SafeVarargs
+    public static PsiJavaElementPattern.Capture<PsiElement> psiJavaElement(final Class<? extends PsiElement>... classAlternatives) {


Nit: the reformat removed final from the other parameters (for example psiJavaElement(Class<T> aClass)), but it's still here and on L84, L98 and L117. Effectively-final is enough for capture in the anonymous classes.

Suggested change

public static PsiJavaElementPattern.Capture<PsiElement> psiJavaElement(final Class<? extends PsiElement>... classAlternatives) {

public static PsiJavaElementPattern.Capture<PsiElement> psiJavaElement(Class<? extends PsiElement>... classAlternatives) {

github-actions · 2026-10-08T19:16:28Z

+        return psiLiteral(null);
+    }
+
+    public static PsiJavaElementPattern.Capture<PsiLiteral> psiLiteral(@Nullable final ElementPattern value) {


Suggested change

public static PsiJavaElementPattern.Capture<PsiLiteral> psiLiteral(@Nullable final ElementPattern value) {

public static PsiJavaElementPattern.Capture<PsiLiteral> psiLiteral(@Nullable ElementPattern value) {

github-actions · 2026-10-08T19:16:29Z

+                    .accepts(myPosition)) {
+                    PsiMethod method = PsiTreeUtil.getParentOfType(myPosition, PsiMethod.class, false, PsiClass.class);
+                    assert method != null;
+                    final boolean hasParams = superConstructorHasParameters(method);


Nit: this final is a leftover; the PR drops final elsewhere, and effectively-final is enough for the anonymous ParenthesesInsertHandler.

Suggested change

final boolean hasParams = superConstructorHasParameters(method);

boolean hasParams = superConstructorHasParameters(method);

github-actions · 2026-10-08T19:16:29Z

+            || isStatementPosition(position);
+    }
+
+    private boolean isLambdaParameterType() {


@RequiredReadAction was added to almost every PSI-touching method in this file, but a few were missed: isLambdaParameterType, getFileForDeclaration (L741), expectsClassLiteral (L993), isForLoopMachinery (L1098) and superConstructorHasParameters (L1146). Some of them are called from annotated methods, so the read-action inspection may flag the gaps.

Suggested change

private boolean isLambdaParameterType() {

@RequiredReadAction

private boolean isLambdaParameterType() {

github-actions · 2026-10-08T19:16:29Z

-      if (expressionPosition && !session.isKeywordAlreadyProcessed(PsiKeyword.VOID)) {
-        result.accept(BasicExpressionCompletionContributor.createKeywordLookupItem(position, PsiKeyword.VOID));
-      }
+    private static boolean expectsClassLiteral(PsiElement position) {


Suggested change

private static boolean expectsClassLiteral(PsiElement position) {

@RequiredReadAction

private static boolean expectsClassLiteral(PsiElement position) {

github-actions · 2026-10-08T19:16:29Z

-    if (isEndOfBlock(position) &&
-      PsiTreeUtil.getParentOfType(position, PsiCodeBlock.class, true, PsiMember.class) != null) {
-      return !isForLoopMachinery(position);
+    private static boolean isForLoopMachinery(PsiElement position) {


Suggested change

private static boolean isForLoopMachinery(PsiElement position) {

@RequiredReadAction

private static boolean isForLoopMachinery(PsiElement position) {

github-actions · 2026-10-08T19:16:29Z

-    if (psiClass == null) {
-      return false;
-    }
+    private static boolean superConstructorHasParameters(PsiMethod method) {


Suggested change

private static boolean superConstructorHasParameters(PsiMethod method) {

@RequiredReadAction

private static boolean superConstructorHasParameters(PsiMethod method) {

Refactoring JavaKeywordCompletion and PsiJavaPatterns.

30473e9

github-actions Bot reviewed Oct 8, 2026

View reviewed changes

VISTALL merged commit 64a0cd3 into consulo:master Oct 8, 2026
1 check passed

unv-unv deleted the kw-completion-refactoring branch October 8, 2026 20:03

	public static PsiJavaElementPattern.Capture<PsiElement> psiJavaElement(final Class<? extends PsiElement>... classAlternatives) {
	public static PsiJavaElementPattern.Capture<PsiElement> psiJavaElement(Class<? extends PsiElement>... classAlternatives) {

	final boolean hasParams = superConstructorHasParameters(method);
	boolean hasParams = superConstructorHasParameters(method);

	private boolean isLambdaParameterType() {
	@RequiredReadAction
	private boolean isLambdaParameterType() {

	private static boolean expectsClassLiteral(PsiElement position) {
	@RequiredReadAction
	private static boolean expectsClassLiteral(PsiElement position) {

	private static boolean isForLoopMachinery(PsiElement position) {
	@RequiredReadAction
	private static boolean isForLoopMachinery(PsiElement position) {

	private static boolean superConstructorHasParameters(PsiMethod method) {
	@RequiredReadAction
	private static boolean superConstructorHasParameters(PsiMethod method) {

Conversation

unv-unv commented Oct 8, 2026

Uh oh!

unv-unv commented Oct 8, 2026

Uh oh!

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

Uh oh!

Summary

Inline comments

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Oct 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Oct 8, 2026 •

edited

Loading