Skip to content

IDEA-113632 - Fix Geb Page content not being available#2862

Open
Poundex wants to merge 1 commit intoJetBrains:masterfrom
Poundex:fix/IDEA-113632-geb-page-content
Open

IDEA-113632 - Fix Geb Page content not being available#2862
Poundex wants to merge 1 commit intoJetBrains:masterfrom
Poundex:fix/IDEA-113632-geb-page-content

Conversation

@Poundex
Copy link
Copy Markdown

@Poundex Poundex commented Oct 15, 2024

Fixes IDEA-113632

When using Page objects, Geb makes the content from the Page definition available. However, IJ is currently not recognising this:

notworking

This change makes content from the current detected Page available:

working

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:

completion

}
if (examineThis instanceof GrExpression call && !(examineThis instanceof GrReferenceExpression)) {
PsiType ret = call.getType();
if (ret != null && pageType.isAssignableFrom(ret) && ret instanceof PsiImmediateClassType ct) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


private static final TestLibrary LIB_GEB = new RepositoryTestLibrary(
'org.codehaus.geb:geb-core:0.7.2',
'org.gebish:geb-core:7.0',
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Comment on lines +288 to +291
myFixture.enableInspections(GroovyAssignabilityCheckInspection)

myFixture.configureByText("SomeSpec.groovy", """
class SomeSpec extends geb.spock.GebSpec {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, don't write tests using groovy language, because we have completely removed it from codebase recently

}

private static void contributePageContent(PsiScopeProcessor processor, ResolveState state, PsiElement place) {
if(place instanceof GrReferenceExpressionImpl expr
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, why not just place instanceof GrReferenceExpression?

Comment on lines +95 to +103
PsiElement findCall = examineThis.getLastChild();
while(findCall != null) {
if(findCall instanceof GrExpression) {
examineThis = findCall;
break;
}
findCall = findCall.getLastChild();
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, replace to examineThis instanceof GrMethodCall

}


private static @Nullable PsiClass findPageChange(PsiElement place, PsiClassType pageType) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment on lines +88 to +90
PsiClass ourPage = null;
while (currentElement != null) {
PsiElement examineThis = currentElement;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()) {
   ...
}

@Hardes1 Hardes1 assigned Hardes1 and unassigned BartvHelvert Nov 15, 2024
@batya239 batya239 added the Waiting for Reply The author needs to provide additional details requested by IntelliJ IDEA developers label Sep 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Waiting for Reply The author needs to provide additional details requested by IntelliJ IDEA developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants