IDEA-113632 - Fix Geb Page content not being available #2862
Conversation
| } | ||
| if (examineThis instanceof GrExpression call && !(examineThis instanceof GrReferenceExpression)) { | ||
| PsiType ret = call.getType(); | ||
| if (ret != null && pageType.isAssignableFrom(ret) && ret instanceof PsiImmediateClassType ct) { |
There was a problem hiding this comment.
Sorry, I think that check ret instanceof PsiImmediateClassType ct is exhaustive, because you already know that your return type is assignable to the geb.Page
| @@ -15,7 +15,7 @@ import static org.jetbrains.plugins.groovy.GroovyProjectDescriptors.LIB_GROOVY_1 | |||
| class GebTestsTest extends LightJavaCodeInsightFixtureTestCase { | |||
|
|
|||
| private static final TestLibrary LIB_GEB = new RepositoryTestLibrary( | |||
| 'org.codehaus.geb:geb-core:0.7.2', | |||
| 'org.gebish:geb-core:7.0', | |||
There was a problem hiding this comment.
Please, use the version of library, when the feature was firstly available, I think it should be org.gebish:geb-core:0.9.0. Also, can you update to the same the other dependencies to the same version?
| myFixture.enableInspections(GroovyAssignabilityCheckInspection) | ||
|
|
||
| myFixture.configureByText("SomeSpec.groovy", """ | ||
| class SomeSpec extends geb.spock.GebSpec { |
There was a problem hiding this comment.
Please, don't write tests using groovy language, because we have completely removed it from codebase recently
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| private static void contributePageContent(PsiScopeProcessor processor, ResolveState state, PsiElement place) { | ||
| if(place instanceof GrReferenceExpressionImpl expr |
There was a problem hiding this comment.
Sorry, why not just place instanceof GrReferenceExpression?
| PsiElement findCall = examineThis.getLastChild(); | ||
| while(findCall != null) { | ||
| if(findCall instanceof GrExpression) { | ||
| examineThis = findCall; | ||
| break; | ||
| } | ||
| findCall = findCall.getLastChild(); | ||
| } | ||
| } |
There was a problem hiding this comment.
Sorry, This construction looks really strange to me. If I understood correctly, you are trying to retrieve the initializer expression of the last variable. To achieve such behaviour, there is more concise approach. You need to retrieve variables by GrVariableDeclaration#getVariables. And from there you can get GrVariable#getInitializerGroovy. Also, I don't understand why you check only last variable, although there can be multiple in the same line.
| findCall = findCall.getLastChild(); | ||
| } | ||
| } | ||
| if (examineThis instanceof GrExpression call && !(examineThis instanceof GrReferenceExpression)) { |
There was a problem hiding this comment.
Please, replace to examineThis instanceof GrMethodCall
| } | ||
|
|
||
|
|
||
| private static @Nullable PsiClass findPageChange(PsiElement place, PsiClassType pageType) { |
There was a problem hiding this comment.
Please, add @Nullable/@NotNull to the parameters
| if(place instanceof GrReferenceExpressionImpl expr | ||
| && !(expr.getParent() instanceof GrMethodCall)) { | ||
|
|
||
| PsiClassType gebPageType = PsiType.getTypeByName("geb.Page", place.getProject(), place.getResolveScope()); |
There was a problem hiding this comment.
We already have PsiClass for geb.Page in GebUtil#contributeMembersInsideTest. Could you, please, create type from it by using TypesUtils#createType to avoid fq name dublication
| PsiClass ourPage = null; | ||
| while (currentElement != null) { | ||
| PsiElement examineThis = currentElement; |
There was a problem hiding this comment.
Please, rename ourPage and examineThis, I doubt, if it is descriptive. Usually, we name inheritors of PsiElement according to their types.
| private static @Nullable PsiClass findPageChange(PsiElement place, PsiClassType pageType) { | ||
| PsiElement currentElement = place; | ||
| PsiClass ourPage = null; | ||
| while (currentElement != null) { |
There was a problem hiding this comment.
It seems that this method will be invoked for each reference inside the method, but you actually traverse the whole method starting from the innermost block to the outermost. Could you, please, rewrite it in the way like:
GrCodeBlock block = PsiTreeUtil.getParentOfType(place, GrCodeBlock.class):
for (var statement : block.getStatements()) {
...
}
Fixes IDEA-113632
When using Page objects, Geb makes the content from the Page definition available. However, IJ is currently not recognising this:
This change makes content from the current detected Page available:
Any method which changes the Page (
to(),via(),page()) or asserts the current page (at()) will modify the current page, and only content from the last detected page will be in scope. The dynamic variables have the correct type, and navigate correctly back to their definitions in the Page. Completion is also working as expected: