Skip to content

Commit

Permalink
Merge pull request #3407 from quantified-uncertainty/if-without-else-…
Browse files Browse the repository at this point in the history
…check

Warning for if without else
  • Loading branch information
OAGr authored Oct 5, 2024
2 parents 6205cbd + 6ea548f commit 134069a
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 12 deletions.
71 changes: 69 additions & 2 deletions packages/ai/__tests__/squiggleCodeWarnings_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ simulateExtinctionRisk() = {
P_Misalignment = P_Misalignment_0 * exp(-K_align * E_align(t))
})
}
`;
`;
const warnings = checkCapitalizedVariableNames(code);
expect(warnings.length).toBe(5);
expect(warnings[0].message).toContain(
Expand Down Expand Up @@ -149,17 +149,52 @@ Costs are provided per **full battery cycle** and per **hour of use**.",
expect(warnings.length).toBe(0);
});
});

describe("checkIfWithoutElse", () => {
it("should detect if statements without else", () => {
const code = `
if x > 5 then y = 10
z = if x > 5 then 10
`;
const warnings = checkIfWithoutElse(code);
expect(warnings.length).toBe(1);
expect(warnings[0].message).toContain(
"'if' statement without a corresponding 'else'"
);
});

it("should detect if statements without else, on next line", () => {
const code = `if year < 2023 then
throw("Year must be 2023 or later")
`;
const warnings = checkIfWithoutElse(code);
expect(warnings.length).toBe(1);
expect(warnings[0].message).toContain(
"'if' statement without a corresponding 'else'"
);
});

it("should not detect if statements with else", () => {
const code = `
x = if x > 5 then 10 else 20
`;
const warnings = checkIfWithoutElse(code);
expect(warnings.length).toBe(0);
});

it("should handle multiple if statements correctly", () => {
const code = `a = 3
b = 5
aa = if a > b then 5 else 10
ab = if a > b then a
`;
const warnings = checkIfWithoutElse(code);
expect(warnings.length).toBe(1);
expect(warnings[0].message).toContain(
"'if' statement without a corresponding 'else'"
);
expect(warnings[0].lineNumber).toBe(4); // Assuming line numbers start at 1
});
});

describe("checkMultipleExpects", () => {
Expand Down Expand Up @@ -188,5 +223,37 @@ Costs are provided per **full battery cycle** and per **hour of use**.",
expect(warnings.length).toBe(1);
expect(warnings[0].message).toContain("Adjacent expect statements found");
});

it("should not detect non-adjacent expect statements", () => {
const code = `
expect(x).toBe(5)
expect(y).toBe(10)
`;
const warnings = checkAdjacentExpectStatements(code);
expect(warnings.length).toBe(0);
});

it("should handle multiple adjacent expect statements correctly", () => {
const code = `
expect(a).toBe(1)
expect(b).toBe(2)
expect(c).toBe(3)
`;
const warnings = checkAdjacentExpectStatements(code);
expect(warnings.length).toBe(2);
expect(warnings[0].message).toContain("Adjacent expect statements found");
expect(warnings[1].message).toContain("Adjacent expect statements found");
});

it("should handle mixed expect and non-expect lines", () => {
const code = `
expect(a).toBe(1)
const b = 2
expect(c).toBe(3)
`;
const warnings = checkAdjacentExpectStatements(code);
expect(warnings.length).toBe(0);
});
});
});
19 changes: 9 additions & 10 deletions packages/ai/src/squiggle/squiggleCodeWarnings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -278,25 +278,24 @@ export function checkCapitalizedVariableNames(code: string): Warning[] {
export function checkIfWithoutElse(code: string): Warning[] {
const warnings: Warning[] = [];
const lines = code.split("\n");
const ifPattern = /^\s*if\s+.*\s+then\s+/;
const elsePattern = /^\s*else\s+/;
const ifPattern = /^\s*(\w+\s*=\s*)?if\s+/;

for (let i = 0; i < lines.length; i++) {
const trimmedLine = lines[i].trim();
if (ifPattern.test(trimmedLine)) {
let hasElse = false;
// Check the next few lines for an else statement
for (let j = i + 1; j < Math.min(i + 5, lines.length); j++) {
if (elsePattern.test(lines[j].trim())) {
hasElse = true;
break;
}
let hasElse = trimmedLine.includes("else");

// Check the next two lines for 'else' if not found on the current line
for (let j = 1; j <= 2 && !hasElse && i + j < lines.length; j++) {
const nextLine = lines[i + j].trim();
hasElse = nextLine.includes("else");
}

if (!hasElse) {
warnings.push({
type: "CHECK_IF_WITHOUT_ELSE",
lineNumber: i + 1,
message: `Line ${i + 1}: 'if' statement without a corresponding 'else'.`,
message: `Line ${i + 1}: 'if' statement without a corresponding 'else'. In Squiggle, every 'if' must have an 'else' clause.`,
});
}
}
Expand Down

0 comments on commit 134069a

Please sign in to comment.