From 0aa69d1aa01266d777ffb7ea2a95713d0784bd60 Mon Sep 17 00:00:00 2001 From: Max Medvedev Date: Wed, 31 Oct 2012 23:49:27 +0400 Subject: [PATCH] fix NPE while creating constructor from super if it has optional params --- .../GroovyGenerateConstructorHandler.java | 22 +++---- .../psi/impl/synthetic/GrLightParameter.java | 19 ++++-- .../impl/synthetic/GrLightTypeElement.java | 57 ++++++++++++++++ .../impl/synthetic/GrReflectedMethodImpl.java | 6 +- .../generate/GroovyGenerateMembersTest.groovy | 66 ++++++++++++++----- 5 files changed, 133 insertions(+), 37 deletions(-) create mode 100644 plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrLightTypeElement.java diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/actions/generate/constructors/GroovyGenerateConstructorHandler.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/actions/generate/constructors/GroovyGenerateConstructorHandler.java index c2a94b4e9cea..f895beb22bb1 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/actions/generate/constructors/GroovyGenerateConstructorHandler.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/actions/generate/constructors/GroovyGenerateConstructorHandler.java @@ -54,18 +54,19 @@ public class GroovyGenerateConstructorHandler extends GenerateConstructorHandler for (ClassMember classMember : classMembers) { if (classMember instanceof PsiMethodMember) { final PsiMethod method = ((PsiMethodMember)classMember).getElement(); - final PsiMethod copy = (PsiMethod)method.copy(); - LOG.assertTrue(copy != null, method.getClass().getName()); - if (copy instanceof GrMethod) { - for (GrParameter parameter : ((GrMethod)copy).getParameterList().getParameters()) { - if (parameter.getTypeElementGroovy() == null) { - parameter.setName(DEF_PSEUDO_ANNO + parameter.getName()); + PsiMethod copy = factory.createMethodFromText(GroovyToJavaGenerator.generateMethodStub(method), method); + if (method instanceof GrMethod) { + GrParameter[] parameters = ((GrMethod)method).getParameterList().getParameters(); + PsiParameter[] copyParameters = copy.getParameterList().getParameters(); + for (int i = 0; i < parameters.length; i++) { + if (parameters[i].getTypeElementGroovy() == null) { + copyParameters[i].setName(DEF_PSEUDO_ANNO + parameters[i].getName()); } } } - res.add(new PsiMethodMember(factory.createMethodFromText(GroovyToJavaGenerator.generateMethodStub(copy), method))); + res.add(new PsiMethodMember(copy)); } else if (classMember instanceof PsiFieldMember) { final PsiField field = ((PsiFieldMember)classMember).getElement(); @@ -85,7 +86,8 @@ public class GroovyGenerateConstructorHandler extends GenerateConstructorHandler } @NotNull - protected List generateMemberPrototypes(PsiClass aClass, ClassMember[] members) throws IncorrectOperationException { + protected List generateMemberPrototypes(PsiClass aClass, ClassMember[] members) + throws IncorrectOperationException { final List list = super.generateMemberPrototypes(aClass, members); List> grConstructors = new ArrayList>(); @@ -93,7 +95,7 @@ public class GroovyGenerateConstructorHandler extends GenerateConstructorHandler for (GenerationInfo generationInfo : list) { final PsiMember constructorMember = generationInfo.getPsiMember(); assert constructorMember instanceof PsiMethod; - final PsiMethod constructor = (PsiMethod) constructorMember; + final PsiMethod constructor = (PsiMethod)constructorMember; final PsiCodeBlock block = constructor.getBody(); assert block != null; @@ -127,6 +129,4 @@ public class GroovyGenerateConstructorHandler extends GenerateConstructorHandler public boolean startInWriteAction() { return true; } - - } diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrLightParameter.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrLightParameter.java index 409f2d7cf1a1..8d722e8e7d22 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrLightParameter.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrLightParameter.java @@ -38,11 +38,20 @@ public class GrLightParameter extends LightVariableBuilder imp private volatile boolean myOptional; private volatile GrModifierList myModifierList; private final PsiElement myScope; + private final GrTypeElement myTypeElement; + private final PsiType myTypeGroovy; - public GrLightParameter(@NotNull String name, @NotNull PsiType type, @NotNull PsiElement scope) { - super(scope.getManager(), name, type, GroovyFileType.GROOVY_LANGUAGE); + public GrLightParameter(@NotNull String name, @Nullable PsiType type, @NotNull PsiElement scope) { + super(scope.getManager(), name, getTypeNotNull(type, scope), GroovyFileType.GROOVY_LANGUAGE); myScope = scope; myModifierList = new GrLightModifierList(this); + myTypeGroovy = type; + myTypeElement = type == null ? null : new GrLightTypeElement(type, scope.getManager()); + } + + @NotNull + private static PsiType getTypeNotNull(PsiType type, PsiElement scope) { + return type != null ? type : PsiType.getJavaLangObject(scope.getManager(), scope.getResolveScope()); } @NotNull @@ -58,7 +67,7 @@ public class GrLightParameter extends LightVariableBuilder imp @Override public GrTypeElement getTypeElementGroovy() { - return null; + return myTypeElement; } @Override @@ -99,12 +108,12 @@ public class GrLightParameter extends LightVariableBuilder imp @Override public PsiType getTypeGroovy() { - return getType(); + return myTypeGroovy; } @Override public PsiType getDeclaredType() { - return getType(); + return myTypeGroovy; } @Override diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrLightTypeElement.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrLightTypeElement.java new file mode 100644 index 000000000000..14e285bfb793 --- /dev/null +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrLightTypeElement.java @@ -0,0 +1,57 @@ +/* + * Copyright 2000-2012 JetBrains s.r.o. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance withe law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.jetbrains.plugins.groovy.lang.psi.impl.synthetic; + +import com.intellij.psi.PsiElement; +import com.intellij.psi.PsiManager; +import com.intellij.psi.PsiType; +import com.intellij.psi.impl.light.LightElement; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.plugins.groovy.GroovyFileType; +import org.jetbrains.plugins.groovy.lang.psi.GroovyElementVisitor; +import org.jetbrains.plugins.groovy.lang.psi.api.types.GrTypeElement; + +/** + * @author Max Medvedev + */ +public class GrLightTypeElement extends LightElement implements GrTypeElement { + @NotNull private final PsiType myType; + + public GrLightTypeElement(@NotNull PsiType type, PsiManager manager) { + super(manager, GroovyFileType.GROOVY_LANGUAGE); + myType = type; + } + + @NotNull + @Override + public PsiType getType() { + return myType; + } + + @Override + public PsiType getTypeNoResolve(PsiElement context) { + return myType; + } + + @Override + public void accept(GroovyElementVisitor visitor) { + visitor.visitTypeElement(this); + } + + @Override + public void acceptChildren(GroovyElementVisitor visitor) { + } + + @Override + public String toString() { + return "light type element"; + } +} diff --git a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrReflectedMethodImpl.java b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrReflectedMethodImpl.java index 825e7d55e471..3e306da30f23 100644 --- a/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrReflectedMethodImpl.java +++ b/plugins/groovy/src/org/jetbrains/plugins/groovy/lang/psi/impl/synthetic/GrReflectedMethodImpl.java @@ -67,7 +67,7 @@ public class GrReflectedMethodImpl extends LightMethodBuilder implements GrRefle new GrLightModifierList(baseMethod), new LightReferenceListBuilder(baseMethod.getManager(), baseMethod.getLanguage(), null), new LightTypeParameterListBuilder(baseMethod.getManager(), baseMethod.getLanguage()) ); - + initParameterList(baseMethod, optionalParams, categoryType); initTypeParameterList(baseMethod); initModifiers(baseMethod, categoryType != null); @@ -99,7 +99,7 @@ public class GrReflectedMethodImpl extends LightMethodBuilder implements GrRefle myModifierList.addModifier(modifier); } } - + for (PsiElement modifier : baseMethod.getModifierList().getModifiers()) { if (modifier instanceof GrAnnotation) { final String qualifiedName = ((GrAnnotation)modifier).getQualifiedName(); @@ -135,7 +135,7 @@ public class GrReflectedMethodImpl extends LightMethodBuilder implements GrRefle } optionalParams--; } - parameterList.addParameter(new GrLightParameter(parameter.getName(), parameter.getType(), this)); + parameterList.addParameter(new GrLightParameter(parameter.getName(), parameter.getDeclaredType(), this)); } LOG.assertTrue(optionalParams == 0, optionalParams + "methodText: " + baseMethod.getText()); diff --git a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/actions/generate/GroovyGenerateMembersTest.groovy b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/actions/generate/GroovyGenerateMembersTest.groovy index 25a7b18ae98b..6e348627c450 100644 --- a/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/actions/generate/GroovyGenerateMembersTest.groovy +++ b/plugins/groovy/test/org/jetbrains/plugins/groovy/lang/actions/generate/GroovyGenerateMembersTest.groovy @@ -46,8 +46,7 @@ public class GroovyGenerateMembersTest extends LightCodeInsightFixtureTestCase { doConstructorTest(); } - - public void testConstructorInJavaInheritor() throws Exception { + public void testConstructorInJavaInheritor() { myFixture.configureByText "GrBase.groovy", """ abstract class GrBase { GrBase(int i) { } @@ -296,16 +295,15 @@ class Test { } void testConstructorInTheMiddle() { - myFixture.configureByText("a.groovy", """ + doConstructorTest """\ class Foo { def foo() {} def bar() {} -}""") - generateConstructor() - myFixture.checkResult """ +} +""", """\ class Foo { def foo() {} @@ -313,13 +311,36 @@ class Foo { } def bar() {} -}""" +} +""" + } + + void testConstructorWithOptionalParameter() { + doConstructorTest('''\ +class Base { + Base(int x = 0){} +} + +class Inheritor extends Base { + +} +''', '''\ +class Base { + Base(int x = 0){} +} + +class Inheritor extends Base { + Inheritor(int x) { + super(x) + } +} +''') } private void generateGetter() { //noinspection GroovyResultOfObjectAllocationIgnored new GroovyGenerateGetterSetterAction() //don't remove it!!! - new WriteCommandAction(project, new PsiFile[0]) { + new WriteCommandAction(project, PsiFile.EMPTY_ARRAY) { protected void run(Result result) throws Throwable { new GenerateGetterHandler() { @Nullable @@ -335,7 +356,7 @@ class Foo { private void generateSetter() { //noinspection GroovyResultOfObjectAllocationIgnored new GroovyGenerateGetterSetterAction() //don't remove it!!! - new WriteCommandAction(project, new PsiFile[0]) { + new WriteCommandAction(project, PsiFile.EMPTY_ARRAY) { protected void run(Result result) throws Throwable { new GenerateSetterHandler() { @Nullable @@ -348,24 +369,36 @@ class Foo { }.execute() } - private void doConstructorTest() { - myFixture.configureByFile(getTestName(false) + ".groovy"); + private void doConstructorTest(String before = null, String after = null) { + if (before != null) { + myFixture.configureByText('_a.groovy', before) + } + else { + myFixture.configureByFile(getTestName(false) + ".groovy"); + } generateConstructor(); - myFixture.checkResultByFile(getTestName(false) + "_after.groovy"); + if (after != null) { + myFixture.checkResult(after) + } + else { + myFixture.checkResultByFile(getTestName(false) + "_after.groovy"); + } } private RunResult generateConstructor(boolean javaHandler = false) { GenerateMembersHandlerBase handler if (javaHandler) { handler = new GenerateConstructorHandler() { - @Override protected ClassMember[] chooseMembers(ClassMember[] members, boolean allowEmptySelection, boolean copyJavadocCheckbox, Project project, Editor editor) { + @Override + protected ClassMember[] chooseMembers(ClassMember[] members, boolean allowEmptySelection, boolean copyJavadocCheckbox, Project project, Editor editor) { return members; } } } else { handler = new GroovyGenerateConstructorHandler() { - @Override protected ClassMember[] chooseOriginalMembersImpl(PsiClass aClass, Project project) { + @Override + protected ClassMember[] chooseOriginalMembersImpl(PsiClass aClass, Project project) { List members = aClass.fields.collect { new PsiFieldMember(it) } members << new PsiMethodMember(aClass.superClass.constructors[0]) return members as ClassMember[] @@ -381,8 +414,5 @@ class Foo { }.execute() } - @Override - protected String getBasePath() { - return TestUtils.getTestDataPath() + "generate"; - } + final String basePath = TestUtils.testDataPath + "generate" }