Fix removal of whitespace between invalid tokens in formatter #41417

poorna2152 · 2023-09-20T06:46:06Z

Purpose

Describe the problems, issues, or needs driving this feature/fix and include links to related issues.

Approach

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

In the case of invalid tokens this indents the code as necessary and preserves the formatting done by the user and stops removal of whitespaces.

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

codecov · 2023-09-20T07:55:35Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b2ac9a9) 0.00% compared to head (5943864) 76.33%.
Report is 14 commits behind head on master.

❗ Current head 5943864 differs from pull request most recent head 80ef085. Consider uploading reports for the commit 80ef085 to get more accurate results

Additional details and impacted files

@@              Coverage Diff              @@
##             master   #41417       +/-   ##
=============================================
+ Coverage      0.00%   76.33%   +76.33%     
- Complexity        0    52488    +52488     
=============================================
  Files             9     2881     +2872     
  Lines            35   198462   +198427     
  Branches          0    25802    +25802     
=============================================
+ Hits              0   151496   +151496     
- Misses           35    38604    +38569     
- Partials          0     8362     +8362

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

LakshanWeerasinghe · 2023-09-21T06:53:27Z

.../formatter/modules/formatter-core/src/test/resources/actions/query/assert/query_action_4.bal

+public function main() {
+    var c = from var i in 0 ... 5
+        select i
+        where i %2 != 0;


Suggested change

where i %2 != 0;

where i % 2 != 0;

Is it possible to make this space?

Are you suggesting that the formatter should add the space after % in this case.

Since this is a Invalid Node and this is a syntax error the formatter is not aware of the context. Thus if the user have not specified a whitespace this does not add a whitespace

Okay. I didn't see there is an syntax error there.

Can you point a test where this formatting is working?

I updated the approach to describe what this PR does and the test added to the PR and the tests skipped here are the tests this PR fixes

IMO, we should keep the formatting of invalid node and its minutiae intact. As it is invalid we should not format it rather skip it from the formatter.

github-actions · 2023-10-07T19:33:09Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

lochana-chathura · 2023-10-17T06:18:06Z

.../formatter/modules/formatter-core/src/test/resources/actions/query/assert/query_action_4.bal

+public function main() {
+    var c = from var i in 0 ... 5
+        select i
+        where i %2 != 0;


IMO, we should keep the formatting of invalid node and its minutiae intact. As it is invalid we should not format it rather skip it from the formatter.

lochana-chathura · 2023-10-17T06:19:55Z

...es/formatter-core/src/main/java/org/ballerinalang/formatter/core/FormattingTreeModifier.java

@@ -4346,7 +4346,9 @@ private MinutiaeList getTrailingMinutiae(Token token) {

        // Preserve the necessary trailing minutiae coming from the original token
        int consecutiveNewlines = 0;
-        for (Minutiae minutiae : token.trailingMinutiae()) {
+        int size = token.trailingMinutiae().size();


I think we need to handle this for leading minutiae as well. In the parser, we attach invalid nodes to both leading and trailing minutiae.

In the case of leading minutiae having invalid nodes the removal of all the whitespace between tokens does not happen. In this case this code section is invoked . And in the both of the branches of the if condition it adds at least one whitespace between the tokens. The getPreservedIndentation function also returns an integer greater than 0

github-actions · 2023-11-02T19:33:06Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

github-actions · 2023-11-18T19:33:06Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

github-actions · 2023-12-04T19:33:18Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

github-actions · 2023-12-20T19:33:11Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

github-actions · 2024-01-05T19:33:18Z

This PR has been open for more than 15 days with no activity. This will be closed in 3 days unless the stale label is removed or commented.

lochana-chathura

Let's sync the PR with master

...es/formatter-core/src/main/java/org/ballerinalang/formatter/core/FormattingTreeModifier.java

poorna2152 requested a review from hevayo as a code owner September 20, 2023 06:46

poorna2152 changed the title ~~Fix removal of whitespace between invalid tokens~~ Fix removal of whitespace between invalid tokens in formatter Sep 20, 2023

poorna2152 added Team/CompilerFE All issues related to Language implementation and Compiler, this exclude run times. Area/Formatting labels Sep 21, 2023

lochana-chathura self-requested a review September 21, 2023 04:08

LakshanWeerasinghe reviewed Sep 21, 2023

View reviewed changes

github-actions bot added the Stale label Oct 7, 2023

poorna2152 removed the Stale label Oct 8, 2023

lochana-chathura reviewed Oct 17, 2023

View reviewed changes

github-actions bot added the Stale label Nov 2, 2023

poorna2152 removed the Stale label Nov 3, 2023

github-actions bot added the Stale label Nov 18, 2023

poorna2152 removed the Stale label Nov 19, 2023

github-actions bot added the Stale label Dec 4, 2023

poorna2152 removed the Stale label Dec 5, 2023

github-actions bot added the Stale label Dec 20, 2023

poorna2152 removed the Stale label Dec 21, 2023

github-actions bot added the Stale label Jan 5, 2024

poorna2152 removed the Stale label Jan 6, 2024

lochana-chathura reviewed Jan 9, 2024

View reviewed changes

...es/formatter-core/src/main/java/org/ballerinalang/formatter/core/FormattingTreeModifier.java Outdated Show resolved Hide resolved

poorna2152 added 3 commits January 9, 2024 10:50

Fix removal of ws between invalid tokens

c53b42c

Add a comment with the approach on ws.

b29b5e3

Update the comment

80ef085

poorna2152 force-pushed the ws_in_invalid_tokens branch from 9507c9f to 80ef085 Compare January 9, 2024 05:28

lochana-chathura approved these changes Jan 9, 2024

View reviewed changes

lochana-chathura merged commit 9e00ea7 into ballerina-platform:master Jan 9, 2024
16 checks passed

poorna2152 added this to the 2201.9.0 milestone Mar 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix removal of whitespace between invalid tokens in formatter #41417

Fix removal of whitespace between invalid tokens in formatter #41417

poorna2152 commented Sep 20, 2023 •

edited

Loading

codecov bot commented Sep 20, 2023 •

edited

Loading

LakshanWeerasinghe Sep 21, 2023

poorna2152 Sep 21, 2023

poorna2152 Sep 21, 2023

LakshanWeerasinghe Sep 22, 2023

LakshanWeerasinghe Sep 22, 2023 •

edited

Loading

poorna2152 Sep 22, 2023

lochana-chathura Oct 17, 2023

github-actions bot commented Oct 7, 2023

lochana-chathura Oct 17, 2023

lochana-chathura Oct 17, 2023

poorna2152 Oct 18, 2023 •

edited

Loading

github-actions bot commented Nov 2, 2023

github-actions bot commented Nov 18, 2023

github-actions bot commented Dec 4, 2023

github-actions bot commented Dec 20, 2023

github-actions bot commented Jan 5, 2024

lochana-chathura left a comment

Fix removal of whitespace between invalid tokens in formatter #41417

Fix removal of whitespace between invalid tokens in formatter #41417

Conversation

poorna2152 commented Sep 20, 2023 • edited Loading

Purpose

Approach

Samples

Remarks

Check List

codecov bot commented Sep 20, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LakshanWeerasinghe Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Oct 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poorna2152 Oct 18, 2023 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Nov 2, 2023

github-actions bot commented Nov 18, 2023

github-actions bot commented Dec 4, 2023

github-actions bot commented Dec 20, 2023

github-actions bot commented Jan 5, 2024

lochana-chathura left a comment

Choose a reason for hiding this comment

poorna2152 commented Sep 20, 2023 •

edited

Loading

codecov bot commented Sep 20, 2023 •

edited

Loading

LakshanWeerasinghe Sep 22, 2023 •

edited

Loading

poorna2152 Oct 18, 2023 •

edited

Loading