Skip to content

Commit

Permalink
Fix edge cases in formatting comments after braces
Browse files Browse the repository at this point in the history
The ASTPrinter did not insert a newline properly causing the single line comment to "eat"'
the next token.

Broke: https://makerworld.com/en/models/245569#
  • Loading branch information
alufers committed Oct 27, 2024
1 parent 0ccce1c commit 78b63b9
Show file tree
Hide file tree
Showing 3 changed files with 81 additions and 8 deletions.
54 changes: 52 additions & 2 deletions src/ASTPrinter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ describe("ASTPrinter", () => {
}
if (shouldInject) {
const injectionBefore = `/* INJ_${injectId}_B */`;
codeWithInjections += injectionBefore;
codeWithInjections += " " + injectionBefore + " ";
injectedStrings.push(injectionBefore);
injectId++;
}
codeWithInjections += tok.lexeme;
if (shouldInject) {
const injectionAfter = `/* INJ_${injectId}_A */`;
codeWithInjections += injectionAfter;
codeWithInjections += " " + injectionAfter + " ";
injectedStrings.push(injectionAfter);
injectId++;
}
Expand Down Expand Up @@ -316,4 +316,54 @@ translate([-5, -5, -10])
cube([10, 10, 10]);
`);
});

test("It preserves comments in files doing division (harness test)", async () => {
doPreserveTest(`translate([distance1-distance1/2,8.5,0]);`);
});

it("doesn't break code with comment after ending brace", async () => {
const formatted = doFormat(`
{
{
} //end if
}`);

let [ast, errorCollector] = ParsingHelper.parseFile(
new CodeFile("<test>", formatted)
);
errorCollector.throwIfAny();
});

it("Adds newlines between stacked closing braces", async () => {
const formatted = doFormat(`
{
{
}}`);
expect(formatted).not.toContain("}}");
});

it("does not add newlines near the else keyword in ifs", async () => {
const formatted = doFormat(`
if(true) {
} else {
}`);
expect(formatted).toContain("} else {");
});

it("doesn't break code with comment after ending brace of an else statement", async () => {
const formatted = doFormat(`
if(true) {
} // hello
else
// hello
{
}`);

let [ast, errorCollector] = ParsingHelper.parseFile(
new CodeFile("<test>", formatted)
);
errorCollector.throwIfAny();
});
});
10 changes: 6 additions & 4 deletions src/ASTPrinter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,10 @@ export default class ASTPrinter implements ASTVisitor<string> {
}
source += this.stringifyExtraTokens(n.tokens.secondParen);
source += ")";
if (!(n.child instanceof NoopStmt) && !this.breakBetweenModuleInstantations) {
if (
!(n.child instanceof NoopStmt) &&
!this.breakBetweenModuleInstantations
) {
source += " ";
}
if (this.breakBetweenModuleInstantations) {
Expand All @@ -467,8 +470,7 @@ export default class ASTPrinter implements ASTVisitor<string> {
c.firstModuleInstantation = false;
}
this.newLineAfterNextComment("breakBetweenModuleInstantations");
source +=
n.child.accept(c);
source += n.child.accept(c);
} else {
const c = this.copyWithBreakBetweenModuleInstantations(false);
c.firstModuleInstantation = true;
Expand Down Expand Up @@ -561,7 +563,7 @@ export default class ASTPrinter implements ASTVisitor<string> {
}
source += "}";
if (!this.doNotAddNewlineAfterBlockStatement) {
source += this.newLine(false, "afterBlockStmt");
this.newLineAfterNextComment("afterBlockStmt");
}
return source;
}
Expand Down
25 changes: 23 additions & 2 deletions src/Lexer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,27 @@ describe("Lexer", () => {
expect(() => lexTokens(`use <xD`)).toThrowError(LexingError);
});
});
it("lexes braces with comments properly", () => {
const tokens = lexTokens(`
{
{
} //end if
}
`);
expect(tokens.map((t) => t.type)).toEqual([
TokenType.LeftBrace,
TokenType.LeftBrace,
TokenType.RightBrace,
TokenType.RightBrace,
TokenType.Eot,
]);
expect(tokens[3].extraTokens).toHaveLength(2);
expect(tokens[3].extraTokens[0]).toBeInstanceOf(SingleLineComment);
expect((tokens[3].extraTokens[0] as SingleLineComment).contents).toEqual(
`end if`
);
expect(tokens[3].extraTokens[1]).toBeInstanceOf(NewLineExtraToken);
});
it("does not add duplicate extraTokens", () => {
const tokens = lexTokens(`
module indented() {
Expand Down Expand Up @@ -407,10 +428,10 @@ describe("Lexer", () => {
it("generates start and and in spans", () => {
const tokens = lexTokens(`b();`); // 5 spaces
expect(tokens.length).not.toBe(0);
tokens.every(t => {
tokens.every((t) => {
expect(t.span.start).toBeTruthy();
expect(t.span.end).toBeTruthy();
})
});
});
describe.skip("lexing of random files found on the internet", () => {
async function lexFile(path: string) {
Expand Down

0 comments on commit 78b63b9

Please sign in to comment.