Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(ast): add support for LambdaExpr to infer type from return expr type #1011

Merged
merged 5 commits into from
Jul 11, 2022
Merged

feat(ast): add support for LambdaExpr to infer type from return expr type #1011

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1d0a373
infer type from return expr, which would enable assignment to an appr…
zhumin8 Jul 1, 2022
24f05f6
add test.
zhumin8 Jul 6, 2022
1de792f
removed as already defined in Expr interface.
zhumin8 Jul 6, 2022
5f360a7
revert removal of type() override.
zhumin8 Jul 8, 2022
6a898d5
instead of modifying type in builder, set directly to concrete method…
zhumin8 Jul 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/main/java/com/google/api/generator/engine/ast/LambdaExpr.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,7 @@
@AutoValue
public abstract class LambdaExpr implements Expr {
@Override
public TypeNode type() {
// TODO(v2): Support set of FunctionalInterface parameterized on the args and return type,
// which would enable assignment to an appropriate variable.
return TypeNode.VOID;
}
public abstract TypeNode type();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

keeping this despite sonar complains about already defined in Expr interface. It's seems the recommend way for @autovalue as explained in this doc. Also keeps consistent with other expression classes like ValueExpr, CastExpr

Copy link
Collaborator

Choose a reason for hiding this comment

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

You might already considered it, can we just implement the type() method to return the type of ReturnExpr, like return returnExpr().expr().type()? So that we don't have to change any of the code in the builder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, I actually did not think of that. Sounds like a better idea with less changes to existing code. The only thing is we won't be able to set a default value for type(), but that should be fine because ReturnExpr().expr() should always have type associated.
Let me make the changes.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If you want, we can still expose the setType() method and set a default value in the builder() method, but like you said ReturnExpr().expr() should always have type associated so we should never call setType() explicitly.


public abstract ImmutableList<VariableExpr> arguments();

Expand All @@ -46,17 +42,22 @@ public void accept(AstNodeVisitor visitor) {
public static Builder builder() {
return new AutoValue_LambdaExpr.Builder()
.setArguments(Collections.emptyList())
.setBody(Collections.emptyList());
.setBody(Collections.emptyList())
.setType(TypeNode.VOID);
}

@AutoValue.Builder
public abstract static class Builder {
abstract Builder setType(TypeNode type);

public Builder setArguments(VariableExpr... arguments) {
return setArguments(Arrays.asList(arguments));
}

public abstract Builder setArguments(List<VariableExpr> arguments);

abstract ReturnExpr returnExpr();

public abstract Builder setBody(List<Statement> body);

public abstract Builder setReturnExpr(ReturnExpr returnExpr);
Expand All @@ -68,10 +69,13 @@ public Builder setReturnExpr(Expr expr) {
public abstract LambdaExpr autoBuild();

public LambdaExpr build() {
LambdaExpr lambdaExpr = autoBuild();
Preconditions.checkState(
!lambdaExpr.returnExpr().expr().type().equals(TypeNode.VOID),
!returnExpr().expr().type().equals(TypeNode.VOID),
"Lambdas cannot return void-typed expressions.");

setType(returnExpr().expr().type());

LambdaExpr lambdaExpr = autoBuild();
// Must be a declaration.
lambdaExpr.arguments().stream()
.forEach(
Expand Down
10 changes: 10 additions & 0 deletions src/test/java/com/google/api/generator/engine/ast/LambdaExprTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package com.google.api.generator.engine.ast;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;

import java.util.Arrays;
Expand All @@ -27,6 +28,15 @@ public void validLambdaExpr_noArguments() {
.build();
}

@Test
public void validLambdaExpr_inferTypeFromReturnExpr() {
LambdaExpr lambdaExpr =
LambdaExpr.builder()
.setReturnExpr(ValueExpr.withValue(StringObjectValue.withValue("foo")))
.build();
assertEquals(TypeNode.STRING, lambdaExpr.type());
}

@Test
public void validLambdaExpr_severalArguments() {
VariableExpr argOneVarExpr =
Expand Down
17 changes: 17 additions & 0 deletions src/test/java/com/google/api/generator/engine/writer/JavaWriterVisitorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,23 @@ public void writeLambdaExpr_noParameters() {
assertEquals("() -> \"foo\"", writerVisitor.write());
}

@Test
public void writeLambdaExpr_assignToVariable() {
LambdaExpr lambdaExpr =
LambdaExpr.builder()
.setReturnExpr(ValueExpr.withValue(StringObjectValue.withValue("foo")))
.build();
AssignmentExpr assignmentExpr =
AssignmentExpr.builder()
.setVariableExpr(
VariableExpr.withVariable(
Variable.builder().setName("word").setType(TypeNode.STRING).build()))
.setValueExpr(lambdaExpr)
.build();
assignmentExpr.accept(writerVisitor);
assertEquals("word = () -> \"foo\"", writerVisitor.write());
}

@Test
public void writeLambdaExpr_oneParameter() {
VariableExpr argVarExpr =
Expand Down