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.

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

Fix: Format for comprehension expressions #3916

Merged
merged 11 commits into from
May 9, 2023
Merged
1 change: 1 addition & 0 deletions Source/DafnyCore/AST/Expressions/ComprehensionExpr.cs
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,7 @@ public virtual bool SetIndent(int indentBefore, TokenNewIndentCollector formatte
indentBefore = formatter.ReduceBlockiness ? indentBefore : formatter.GetNewTokenVisualIndent(token, indentBefore);
assignOpIndent = formatter.ReduceBlockiness ? indentBefore + formatter.SpaceTab : indentBefore;
formatter.SetOpeningIndentedRegion(token, indentBefore);

break;
}
case ":=":
Expand Down
34 changes: 33 additions & 1 deletion Source/DafnyCore/AST/Expressions/Expressions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Numerics;
using System.Linq;
using System.Diagnostics;
using System.Reflection.Emit;
using System.Security.AccessControl;
using Microsoft.Boogie;

Expand Down Expand Up @@ -2274,7 +2275,11 @@ public bool SetIndent(int indentBefore, TokenNewIndentCollector formatter) {
formatter.SetIndentations(ownedTokens[0], formatter.binOpIndent, formatter.binOpIndent, formatter.binOpArgIndent);
} else {
var startToken = this.StartToken;
var newIndent = formatter.GetNewTokenVisualIndent(startToken, formatter.GetIndentInlineOrAbove(startToken));
//"," in a comprehension is an "&&", except that it should not try to keep a visual indentation between components.
var newIndent =
ownedTokens[0].val == "," ?
formatter.GetIndentInlineOrAbove(startToken)
: formatter.GetNewTokenVisualIndent(startToken, formatter.GetIndentInlineOrAbove(startToken));
formatter.SetIndentations(ownedTokens[0], newIndent, newIndent, newIndent);
}
}
Expand Down Expand Up @@ -2339,6 +2344,33 @@ public bool SetIndent(int indentBefore, TokenNewIndentCollector formatter) {
formatter.Visit(E1, item2Indent);
formatter.SetIndentations(EndToken, below: indent);
return false;
} else if (Op is Opcode.In or Opcode.NotIn) {
var itemIndent = formatter.GetNewTokenVisualIndent(
E0.StartToken, indent);
var item2Indent = itemIndent + formatter.SpaceTab;
formatter.Visit(E0, itemIndent);
foreach (var token in this.OwnedTokens) {
switch (token.val) {
case "<": {
if (token.Prev.line != token.line) {
itemIndent = formatter.GetNewTokenVisualIndent(token, indent);
}

break;
}
case "in":
case "-": {
if (TokenNewIndentCollector.IsFollowedByNewline(token)) {
formatter.SetOpeningIndentedRegion(token, itemIndent);
} else {
formatter.SetAlign(itemIndent, token, out item2Indent, out _);
}
break;
}
}
}
formatter.Visit(E1, item2Indent);
return false;
} else {
foreach (var token in OwnedTokens) {
formatter.SetIndentations(token, indent, indent, indent);
Expand Down
79 changes: 78 additions & 1 deletion Source/DafnyPipeline.Test/FormatterIssues.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,83 @@ namespace DafnyPipeline.Test;

[Collection("Singleton Test Collection - FormatterForTopLevelDeclarations")]
public class FormatterIssues : FormatterBaseTest {
[Fact]
public void GitIssue3912FormatterCollectionArrow() {
FormatterWorksFor(@"
const i :=
a in
B + // Previously it was not indented
C +
D &&
b in
B +
C
");
}

[Fact]
public void GitIssue3912FormatterCollectionArrowA() {
FormatterWorksFor(@"
const newline :=
set
i <-
PartA +
PartB +
PartC,
j <-
PartD
::
f(i,j)

const sameline :=
set i <-
PartA +
PartB +
PartC,
j <-
PartD
::
f(i,j)

");
}

[Fact]
public void GitIssue3912FormatterCollectionArrowB() {
FormatterWorksFor(@"
const newlineOp :=
set
i
<-
PartA +
PartB +
PartC,
j
<-
PartD
::
f(i,j)

");
}

[Fact]
public void GitIssue3912FormatterCollectionArrowC() {
FormatterWorksFor(@"
const sameLineOp :=
set i
<-
PartA +
PartB +
PartC,
j
<- PartD +
PartE
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this PartE intended to be aligned with PartD, or is it supposed to be one 2-space indent past the corresponding <-?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is intended to be aligned with PartD. Specifically, the "<-" is treated like an "in" binary operator, and I put the rule that if the "in" operator is not followed by a newline, then it starts a new aligning indentation. If it is followed by a newline, then it's indented by two spaces as usual. This matches usages of newlines I've seen for the second argument of "in" and "<-"

::
f(i,j)
");
}

[Fact]
public void GitIssue3960FormattingIssueForallStatements() {
FormatterWorksFor(@"
Expand Down Expand Up @@ -50,4 +127,4 @@ public void FormatterWorksForEmptyDocument() {
FormatterWorksFor(@"
", null, true);
}
}
}
1 change: 1 addition & 0 deletions docs/dev/news/3912.fix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Format for comprehension expressions