Skip to content

#2346 redone: a formatter for Rascal itself, plus the necessary improvements and stabilization in the Box formatter framework code.#2738

Open
jurgenvinju wants to merge 20 commits intomainfrom
second-try-rascal-formatter
Open

#2346 redone: a formatter for Rascal itself, plus the necessary improvements and stabilization in the Box formatter framework code.#2738
jurgenvinju wants to merge 20 commits intomainfrom
second-try-rascal-formatter

Conversation

@jurgenvinju
Copy link
Copy Markdown
Member

@jurgenvinju jurgenvinju commented Mar 31, 2026

This is a redo of #2346 which was mangled due to a wrong use of git-filter-repo.

Rascal is a pretty big language so this pushes the new toBox and format and layoutDiff functions to the max.

  • The goal is to formulate toBox rules such that the formatter produces beautiful layout, and
  • We identify bugs and efficiency issues in toBox, format and layoutDiff and fix them immediately.
  • We add box operators with variable argument lists to eliminate a level of superfluous brackets
  • We test for exceptions, parse errors and comment preservation, on all files of the standard library and the larger files in the Compiler and checker.
  • re-implementation (optimized) of U and G
  • integration tests succeed https://github.com/usethesource/rascal-core-big-tests/actions/runs/23592431666

@jurgenvinju jurgenvinju changed the title Another clean PR branch to work with for the formatter" Redo #2346 formatter branch Mar 31, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 31, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 46%. Comparing base (f6d6bbe) to head (d0d02e0).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##              main   #2738   +/-   ##
=======================================
  Coverage       46%     46%           
- Complexity    6725    6732    +7     
=======================================
  Files          794     794           
  Lines        65923   65936   +13     
  Branches      9888    9888           
=======================================
+ Hits         30837   30853   +16     
+ Misses       32696   32693    -3     
  Partials      2390    2390           

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread src/org/rascalmpl/library/analysis/diff/edits/ExecuteTextEdits.rsc Outdated
Comment thread src/org/rascalmpl/library/analysis/diff/edits/HiFiLayoutDiff.rsc Outdated
Comment thread src/org/rascalmpl/library/lang/rascal/format/Rascal.rsc Outdated
Comment thread src/org/rascalmpl/library/lang/rascal/grammar/definition/Priorities.rsc Outdated
Comment thread src/org/rascalmpl/library/util/Highlight.rsc
Comment thread src/org/rascalmpl/library/IO.rsc Outdated
Comment thread src/org/rascalmpl/library/util/Monitor.rsc
Comment thread src/org/rascalmpl/library/List.rsc
Comment thread src/org/rascalmpl/library/Type.rsc
@jurgenvinju
Copy link
Copy Markdown
Member Author

jurgenvinju commented Apr 19, 2026

thanks for the review @toinehartman . Before we merge, some of the TODOs @toinehartman highlighted should be checked. See the conversation above.

@sonarqubecloud
Copy link
Copy Markdown

@jurgenvinju jurgenvinju changed the title Redo #2346 formatter branch #2346 redone: a formatter for Rascal itself, plus the necessary improvements and stabilization in the Box formatter framework code. Apr 20, 2026
@jurgenvinju jurgenvinju self-assigned this Apr 21, 2026
@jurgenvinju
Copy link
Copy Markdown
Member Author

@DavyLandman this seems ready. lots of fixes in Box-related code, fixed in layoutDif, and new features for Rascal formatting.

Copy link
Copy Markdown
Member

@toinehartman toinehartman left a comment

Choose a reason for hiding this comment

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

Nice to see the declarative formatting nearing completion! I just have some small remarks.

// first add required locations to layout nodes
original = reposition(original, markLit=true, markLayout=true, markSubLayout=true);
// TODO: check if indeed repositioning is never needed
// original = reposition(original, markLit=true, markLayout=true, markSubLayout=true);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we do an remove this TODO?

data Row = R(list[Box] cells);

// Row R(Box cells...) = _R(cells);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we still need this?

@pitfalls{
* only useful for debugging purposes, because it becomes a pipeline bottleneck otherwise.
}
Box debUG(Box b) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is the name a funny play on the fact that this is used mostly with U and G boxes?

bool trimTrailingWhitespace = true,
bool insertFinalNewline = true,
bool trimFinalNewlines = true
) = formattingOptions();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I really like this! Looking forward to plugging this into LSP as well.

}

Box toClusterBox(&T* lst, FO opts=fo()) = toClusterBox([e | e <- lst], opts=opts);
Box toClusterBox(&T+ lst, FO opts=fo()) = toClusterBox([e | e <- lst], opts=opts);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Star and plus patterns? I learn something new every day...

@synopsis{Convert the Rascal internal grammar representation format (Grammar) to
a syntax definition in Rascal source code.}
@pitfalls{
This function does not use advanced formatting feature because it is a part of
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
This function does not use advanced formatting feature because it is a part of
This function does not use advanced formatting features because it is a part of

Comment on lines +929 to +945
// Box toBox((Expression) `<Expression a> in <Expression b>`)
// = H1(HOV(toBox(a)), L("in"), HOV(toBox(b)));

// Box toBox((Expression) `<Expression a> \<= <Expression b>`)
// = H1(HOV(toBox(a)), L("\<="), HOV(toBox(b)));

// Box toBox((Expression) `<Expression a> \< <Expression b>`)
// = H1(HOV(toBox(a)), L("\<"), HOV(toBox(b)));

// Box toBox((Expression) `<Expression a> == <Expression b>`)
// = H1(HOV(toBox(a)), L("=="), HOV(toBox(b)));

// Box toBox((Expression) `<Expression a> \>= <Expression b>`)
// = H1(HOV(toBox(a)), L("\>="), HOV(toBox(b)));

// Box toBox((Expression) `<Expression a> \> <Expression b>`)
// = H1(HOV(toBox(a)), L("\>"), HOV(toBox(b)));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Commented code; should we keep this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Does this PR contain a bootstrap of the parser? If so, why?

Comment on lines +318 to +319
* will do nothing if the formatter breaks syntax rules or introduces ambiguity.
* will never lose any comments.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

💯

import IO;

@synopsis{Styles for span classes generated by ((ToHTML))}
public str ToHTMLStyle() =
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we align this with the other names in this module?

Suggested change
public str ToHTMLStyle() =
public str toHTMLStyle() =

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants