[java-intentions] MoveMembersIntoClassFix: improve and revive property test

After implicit classes, out-of-class member is parsed. If it was mistakenly put out of the class, it may have many unrelated compilation errors. E.g. extra Override annotation, unresolved reference (to another member of the class), etc. These errors were masking the 'Move member to class' action, because its scope was larger (the whole method).
Now, we put 'feature not available' error specifically to the method/field name identifier, so it's almost always available. Also, action name is more friendly now, and JavaOutOfClassDefinitionPropertyTest is tuned: caret position and language level set correctly.

GitOrigin-RevId: 39917d11fb216be4db3f6fbc9e984ca7378f7302
This commit is contained in:
Tagir Valeev
2024-04-22 15:43:05 +02:00
committed by intellij-monorepo-bot
parent d057df2905
commit ee495db972
10 changed files with 39 additions and 21 deletions

View File

@@ -620,6 +620,7 @@ error.unnamed.method.parameter.not.allowed=Unnamed method parameter is not allow
error.unnamed.variable.not.allowed.in.this.context=Unnamed variable declaration is not allowed in this context
error.unnamed.variable.brackets=Brackets are not allowed after unnamed variable declaration
error.unnamed.variable.without.initializer=Unnamed variable declaration must have an initializer
intention.name.move.members.into.class=Move {0, choice, 1#member|2#members} into class
intention.family.name.move.members.into.class=Move members into class
chooser.popup.title.select.class.to.move.members.to=Select Target Class
intention.family.name.move.members.to=Move members to {0}

View File

@@ -1402,10 +1402,17 @@ public final class HighlightClassUtil {
return null;
}
HighlightInfo.Builder builder = HighlightUtil.checkFeature(member, JavaFeature.IMPLICIT_CLASSES, languageLevel, psiFile);
PsiElement anchor = member;
if (member instanceof PsiNameIdentifierOwner owner) {
PsiElement nameIdentifier = owner.getNameIdentifier();
if (nameIdentifier != null) {
anchor = nameIdentifier;
}
}
HighlightInfo.Builder builder = HighlightUtil.checkFeature(anchor, JavaFeature.IMPLICIT_CLASSES, languageLevel, psiFile);
if (builder == null) return null;
if (!(member instanceof PsiClass) && !PsiUtil.isAvailable(JavaFeature.IMPLICIT_CLASSES, member)) {
if (!(member instanceof PsiClass)) {
boolean hasClassToRelocate = PsiTreeUtil.findChildOfType(implicitClass, PsiClass.class) != null;
if (hasClassToRelocate) {
MoveMembersIntoClassFix fix = new MoveMembersIntoClassFix(implicitClass);

View File

@@ -655,7 +655,6 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh
@Override
public void visitField(@NotNull PsiField field) {
super.visitField(field);
if (!hasErrorResults()) add(HighlightClassUtil.checkImplicitClassMember(field, myLanguageLevel, myFile));
if (!hasErrorResults()) add(HighlightClassUtil.checkIllegalInstanceMemberInRecord(field));
if (!hasErrorResults()) add(HighlightControlFlowUtil.checkFinalFieldInitialized(field));
}
@@ -688,6 +687,9 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh
public void visitIdentifier(@NotNull PsiIdentifier identifier) {
PsiElement parent = identifier.getParent();
if (parent instanceof PsiVariable variable) {
if (variable instanceof PsiField field) {
add(HighlightClassUtil.checkImplicitClassMember(field, myLanguageLevel, myFile));
}
add(HighlightUtil.checkVariableAlreadyDefined(variable));
if (variable.isUnnamed()) {
HighlightInfo.Builder notAvailable = checkFeature(variable, JavaFeature.UNNAMED_PATTERNS_AND_VARIABLES);
@@ -721,6 +723,7 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh
}
}
else if (parent instanceof PsiMethod method) {
add(HighlightClassUtil.checkImplicitClassMember(method, myLanguageLevel, myFile));
if (method.isConstructor()) {
HighlightInfo.Builder info = HighlightMethodUtil.checkConstructorName(method);
if (info != null) {
@@ -905,7 +908,6 @@ public class HighlightVisitorImpl extends JavaElementVisitor implements Highligh
@Override
public void visitMethod(@NotNull PsiMethod method) {
super.visitMethod(method);
if (!hasErrorResults()) add(HighlightClassUtil.checkImplicitClassMember(method, myLanguageLevel, myFile));
if (!hasErrorResults()) add(HighlightControlFlowUtil.checkUnreachableStatement(method.getBody()));
if (!hasErrorResults()) add(HighlightMethodUtil.checkConstructorHandleSuperClassExceptions(method));
if (!hasErrorResults()) add(HighlightMethodUtil.checkRecursiveConstructorInvocation(method));

View File

@@ -9,6 +9,7 @@ import com.intellij.psi.PsiImplicitClass;
import com.intellij.psi.PsiMember;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.util.containers.ContainerUtil;
import one.util.streamex.StreamEx;
import org.jetbrains.annotations.NonNls;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -28,7 +29,10 @@ public class MoveMembersIntoClassFix extends PsiBasedModCommandAction<PsiImplici
@Override
protected @Nullable Presentation getPresentation(@NotNull ActionContext context, @NotNull PsiImplicitClass implicitClass) {
return Presentation.of(JavaAnalysisBundle.message("intention.family.name.move.members.into.class"));
long count = StreamEx.of(implicitClass.getChildren()).select(PsiMember.class)
.remove(PsiClass.class::isInstance).count();
if (count == 0) return null;
return Presentation.of(JavaAnalysisBundle.message("intention.name.move.members.into.class", count));
}
@Override

View File

@@ -1,8 +1,8 @@
<error descr="Implicitly declared classes are not supported at language level '20'">void foo() {
void <error descr="Implicitly declared classes are not supported at language level '20'">foo</error>() {
}</error>
}
<error descr="Implicitly declared classes are not supported at language level '20'">String s = "asd";</error>
String <error descr="Implicitly declared classes are not supported at language level '20'">s</error> = "asd";
<error descr="Implicitly declared classes are not supported at language level '20'">static {

View File

@@ -1,4 +1,4 @@
// "Move members into class" "true-preview"
// "Move member into class" "true-preview"
class A {

View File

@@ -1,4 +1,4 @@
// "Move members into class" "true-preview"
// "Move member into class" "true-preview"
class A {

View File

@@ -1,4 +1,4 @@
// "Move members into class" "true-preview"
// "Move member into class" "true-preview"
int field<caret> = 12;

View File

@@ -1,4 +1,4 @@
// "Move members into class" "true-preview"
// "Move member into class" "true-preview"
void foo<caret>() {

View File

@@ -8,10 +8,12 @@ import com.intellij.openapi.editor.Document;
import com.intellij.openapi.project.Project;
import com.intellij.openapi.roots.ProjectFileIndex;
import com.intellij.openapi.vfs.VirtualFile;
import com.intellij.pom.java.LanguageLevel;
import com.intellij.psi.*;
import com.intellij.psi.impl.source.tree.ChildRole;
import com.intellij.psi.impl.source.tree.java.FieldElement;
import com.intellij.psi.util.PsiTreeUtil;
import com.intellij.testFramework.IdeaTestUtil;
import com.intellij.testFramework.fixtures.LightJavaCodeInsightFixtureTestCase;
import com.intellij.testFramework.propertyBased.MadTestingAction;
import com.intellij.testFramework.propertyBased.MadTestingUtil;
@@ -32,11 +34,13 @@ import java.util.function.Supplier;
public class JavaOutOfClassDefinitionPropertyTest extends LightJavaCodeInsightFixtureTestCase {
public void testMoveMethodOutOfClassAndBack() {
Supplier<MadTestingAction> fileAction =
MadTestingUtil.performOnFileContents(myFixture, PathManager.getHomePath(), f -> f.getName().endsWith(".java"),
this::doTestMoveMethodOutOfClassAndBack);
PropertyChecker.customized()
.checkScenarios(fileAction);
IdeaTestUtil.withLevel(getModule(), LanguageLevel.JDK_17, () -> {
Supplier<MadTestingAction> fileAction =
MadTestingUtil.performOnFileContents(myFixture, PathManager.getHomePath(), f -> f.getName().endsWith(".java"),
this::doTestMoveMethodOutOfClassAndBack);
PropertyChecker.customized()
.checkScenarios(fileAction);
});
}
private void doTestMoveMethodOutOfClassAndBack(@NotNull ImperativeCommand.Environment env, @NotNull VirtualFile file) {
@@ -55,8 +59,8 @@ public class JavaOutOfClassDefinitionPropertyTest extends LightJavaCodeInsightFi
SmartPsiElementPointer<PsiClass> classPtr = SmartPointerManager.createPointer(psiClass);
if (!moveMemberFromClass(project, psiClass, psiMember)) return;
psiClass = Objects.requireNonNull(classPtr.getElement());
PsiErrorElement errorElement = Objects.requireNonNull(PsiTreeUtil.getPrevSiblingOfType(psiClass, PsiErrorElement.class));
int outerMemberOffset = errorElement.getTextRange().getStartOffset();
PsiMember memberInImplicitClass = Objects.requireNonNull(PsiTreeUtil.getPrevSiblingOfType(psiClass, PsiMember.class));
int outerMemberOffset = memberInImplicitClass.getTextOffset();
assertTrue("Failed to find member after moving it out of a class", outerMemberOffset != -1);
moveMemberToClass(outerMemberOffset);
psiClass = classPtr.getElement();
@@ -121,8 +125,8 @@ public class JavaOutOfClassDefinitionPropertyTest extends LightJavaCodeInsightFi
private static @NotNull List<PsiMember> findMembers(@NotNull PsiClass psiClass) {
Collection<? extends PsiMember> children = PsiTreeUtil.getChildrenOfAnyType(psiClass, PsiMethod.class, PsiField.class);
return ContainerUtil.filter(children, c -> (c instanceof PsiField && !isMultipleFieldsDeclaration((PsiField)c) ||
c instanceof PsiMethod && !((PsiMethod)c).isConstructor()) &&
return ContainerUtil.filter(children, c -> (c instanceof PsiField field && !isMultipleFieldsDeclaration(field) ||
c instanceof PsiMethod method && !method.isConstructor()) &&
// parser works only with simple type params, without nested type params and qualified names
!hasTypeParameters(c)
);