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

Add parser changes to allow annotations for each member in a tuple #36495

Merged
merged 42 commits into from
Aug 4, 2022
Merged

Add parser changes to allow annotations for each member in a tuple #36495

merged 42 commits into from
Aug 4, 2022

Conversation

ushirask
Copy link
Contributor

Purpose

Allow annotations to be added to each member in a tuple

Fixes #36390

Approach

Describe how you are implementing the solutions along with the design details.

Samples

Provide high-level details about the samples related to this feature.

Remarks

List any other known issues, related PRs, TODO items, or any other notes related to the PR.

Check List

  • Read the Contributing Guide
  • Updated Change Log
  • Checked Tooling Support (#)
  • Added necessary tests
    • Unit Tests
    • Spec Conformance Tests
    • Integration Tests
    • Ballerina By Example Tests
  • Increased Test Coverage
  • Added necessary documentation
    • API documentation
    • Module documentation in Module.md files
    • Ballerina By Examples

@ushirask ushirask changed the base branch from master to annot-tuple-members June 12, 2022 15:06
@ushirask ushirask changed the title Allow annotations for each type in a tuple Parser changes to allow annotations for each type in a tuple Jun 12, 2022
@ushirask ushirask changed the title Parser changes to allow annotations for each type in a tuple Parser changes to allow annotations for each member in a tuple Jun 12, 2022
@ushirask ushirask marked this pull request as ready for review June 13, 2022 05:10
lochana-chathura
lochana-chathura reviewed Jun 13, 2022
View reviewed changes
compiler/ballerina-treegen/src/main/resources/syntax_tree_descriptor.json Outdated Show resolved Hide resolved
compiler/ballerina-treegen/src/main/resources/syntax_tree_descriptor.json Outdated Show resolved Hide resolved
compiler/ballerina-treegen/src/main/resources/syntax_tree_descriptor.json Outdated Show resolved Hide resolved
...er/src/main/java/io/ballerina/compiler/internal/parser/tree/STTupleMemberDescriptorNode.java Outdated Show resolved Hide resolved
...lerina-parser/src/main/java/io/ballerina/compiler/syntax/tree/TupleMemberDescriptorNode.java Outdated Show resolved Hide resolved
...lerina-parser/src/main/java/io/ballerina/compiler/syntax/tree/TupleMemberDescriptorNode.java Outdated Show resolved Hide resolved
...lerina-parser/src/main/java/io/ballerina/compiler/syntax/tree/TupleMemberDescriptorNode.java Outdated Show resolved Hide resolved
...-parser/src/test/java/io/ballerinalang/compiler/parser/test/syntax/misc/AnnotationsTest.java Outdated Show resolved Hide resolved
compiler/ballerina-treegen/src/main/resources/syntax_tree_descriptor.json Outdated Show resolved Hide resolved
@ushirask ushirask requested a review from hevayo as a code owner June 13, 2022 09:57
@MaryamZi
Copy link
Member

@ushirask, please fix the PR title to be similar to the commit messages. See #30227 (comment).

@ushirask ushirask changed the title Parser changes to allow annotations for each member in a tuple Add parser changes to allow annotations for each member in a tuple Jun 15, 2022
@ushirask ushirask requested a review from rdulmina July 8, 2022 04:43
lochana-chathura
lochana-chathura reviewed Jul 8, 2022
View reviewed changes
...er/ballerina-parser/src/main/java/io/ballerina/compiler/internal/parser/BallerinaParser.java Outdated Show resolved Hide resolved
...er/ballerina-parser/src/main/java/io/ballerina/compiler/internal/parser/BallerinaParser.java Outdated Show resolved Hide resolved
...er/ballerina-parser/src/main/java/io/ballerina/compiler/internal/parser/BallerinaParser.java Show resolved Hide resolved
...er/ballerina-parser/src/main/java/io/ballerina/compiler/internal/parser/BallerinaParser.java Outdated Show resolved Hide resolved
...er/ballerina-parser/src/main/java/io/ballerina/compiler/internal/parser/BallerinaParser.java Show resolved Hide resolved
...-parser/src/main/java/io/ballerina/compiler/internal/parser/BallerinaParserErrorHandler.java Outdated Show resolved Hide resolved
@ushirask ushirask force-pushed the annot-tuple-each-type branch from 78a7877 to 8aade1a Compare July 14, 2022 06:42
@ushirask ushirask force-pushed the annot-tuple-each-type branch from 5904b41 to 69cb15c Compare July 14, 2022 08:24
lochana-chathura
lochana-chathura reviewed Jul 19, 2022
View reviewed changes
...es/formatter-core/src/main/java/org/ballerinalang/formatter/core/FormattingTreeModifier.java Outdated Show resolved Hide resolved
...es/formatter-core/src/main/java/org/ballerinalang/formatter/core/FormattingTreeModifier.java Outdated Show resolved Hide resolved
...dules/formatter-core/src/test/java/org/ballerinalang/formatter/core/ParserTestFormatter.java Outdated
@@ -172,7 +172,8 @@ public List<String> skipList() {
"predeclared-module-prefix_02.bal", "object_type_def_source_44.bal", "record_type_def_source_27.bal",
"func_type_source_09.bal", "func_type_source_13.bal", "func_type_source_14.bal",
"func_type_source_15.bal", "func_type_source_16.bal", "import_decl_source_24.bal",
"member_access_expr_source_11.bal", "float_literal_source_08.bal", "resiliency_source_05.bal");
"member_access_expr_source_11.bal", "float_literal_source_08.bal", "resiliency_source_05.bal",
"tuple_type_source_12.bal");
Copy link
Member

Choose a reason for hiding this comment

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

Formatter should be able to run on top of syntax errors as well.
Any reason we are skipping tuple_type_source_12.bal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

type T1 [int % @annot string] is formatted to type T1 [int% @annot string] , should this be handled in the formatter?

Copy link
Member

Choose a reason for hiding this comment

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

I see. We have #35240. Since int% get tokenized the same way as int % we can keep this test as int% as well while enabling via the formatter.

Copy link
Contributor Author

@ushirask ushirask Jul 20, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This had to be changed to type T1 [int%   @annot string] (extra space between % and @annot). Does this need to be changed?

Copy link
Member

@lochana-chathura lochana-chathura Jul 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

that's okay for now. Once the aforementioned issue is fixed, this test gets updated anyway.

rdulmina
rdulmina approved these changes Aug 1, 2022
View reviewed changes
Copy link
Contributor

@rdulmina rdulmina left a comment

Choose a reason for hiding this comment

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

Let's update the error message

compiler/ballerina-parser/src/main/resources/syntax_diagnostic_message.properties Show resolved Hide resolved
@MaryamZi MaryamZi merged commit 44c492d into ballerina-platform:annot-tuple-members Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lochana-chathura lochana-chathura lochana-chathura approved these changes

@rdulmina rdulmina rdulmina approved these changes

@MaryamZi MaryamZi Awaiting requested review from MaryamZi

@suleka96 suleka96 Awaiting requested review from suleka96

@chiranSachintha chiranSachintha Awaiting requested review from chiranSachintha

@sameerajayasoma sameerajayasoma Awaiting requested review from sameerajayasoma

@hasithaa hasithaa Awaiting requested review from hasithaa

@hevayo hevayo Awaiting requested review from hevayo

Assignees
No one assigned
Labels
Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow annotations for each type in a tuple
4 participants
@ushirask @MaryamZi @lochana-chathura @rdulmina