Skip to content
This repository was archived by the owner on May 14, 2026. It is now read-only.

[ggj] feat: support box/primitive type equality for ternary expression - #143

Merged
xiaozhenliu-gg5 merged 49 commits into
masterfrom
box-type-ternary
Aug 19, 2020
Merged

[ggj] feat: support box/primitive type equality for ternary expression#143
xiaozhenliu-gg5 merged 49 commits into
masterfrom
box-type-ternary

Conversation

@xiaozhenliu-gg5

@xiaozhenliu-gg5 xiaozhenliu-gg5 commented Jul 29, 2020

Copy link
Copy Markdown
Contributor

[3/6] support boxed/primitive type equality in ternayexpression.

More details can be found in notes: go/javanaut-boxed-primitive-type-equality

@xiaozhenliu-gg5 xiaozhenliu-gg5 added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jul 29, 2020
@xiaozhenliu-gg5 xiaozhenliu-gg5 changed the title [ggj][wip] feat: support box/primitive type equality for ternary expression [ggj] feat: support box/primitive type equality for ternary expression Aug 12, 2020
@xiaozhenliu-gg5 xiaozhenliu-gg5 removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Aug 12, 2020

@miraleung miraleung left a comment

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 remove the CastExpr changes from this PR, and I'll take a second pass then.

Comment thread src/test/java/com/google/api/generator/engine/ast/TernaryExprTest.java Outdated
Comment thread src/main/java/com/google/api/generator/engine/ast/TernaryExpr.java Outdated

@miraleung miraleung left a comment

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 separate this PR from #187.

Comment thread src/main/java/com/google/api/generator/engine/ast/TernaryExpr.java
Comment thread src/main/java/com/google/api/generator/engine/ast/TernaryExpr.java
assertEquals(ternaryExpr.thenExpr().type(), ternaryExpr.elseExpr().type());
assertEquals(ternaryExpr.type(), TypeNode.INT_OBJECT);
}

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.

Could you please confirm that we've covered all the edge cases in these tests?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

sure, added two valid unit tests that ensure: the ternary expression has the correct type with primitive/boxed type expressions in different order.
Also an invalid test to confirm the thenExpr/elseExpr should be primitive/boxed type equal.

@xiaozhenliu-gg5
xiaozhenliu-gg5 merged commit 922c3b8 into master Aug 19, 2020
@xiaozhenliu-gg5
xiaozhenliu-gg5 deleted the box-type-ternary branch September 14, 2020 22:15
suztomo pushed a commit that referenced this pull request Mar 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants