Skip to content

Commit f01fb85

Browse files
fix: correctly parse exp() function
1 parent 561e4fc commit f01fb85

2 files changed

Lines changed: 151 additions & 12 deletions

File tree

web-app/js/projects/calculator.js

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -352,10 +352,34 @@ function initCalculator() {
352352
function compileMathFunction(expr, isGraphing) {
353353
let sanitized = expr.toLowerCase();
354354

355+
// Function names are protected as opaque placeholders BEFORE any
356+
// implicit-multiplication or constant substitution runs. Without
357+
// this, the single-letter matchers for the constants "e" and "x"
358+
// would tokenize *inside* a function name -- e.g. "exp(" would get
359+
// split into "e" + "xp(" and mangled into "Math.E*xp(", which is
360+
// exactly why exp(1) used to fail. Placeholders use a control
361+
// character + uppercase letter so they can never collide with a
362+
// digit, "x", "e", "pi"/"π", or a real parenthesis.
363+
const FUNCS = ['sin', 'cos', 'tan', 'sqrt', 'log', 'ln', 'abs', 'exp'];
364+
const FUNC_REPLACEMENTS = [
365+
'Math.sin(', 'Math.cos(', 'Math.tan(', 'Math.sqrt(',
366+
'Math.log10(', 'Math.log(', 'Math.abs(', 'Math.exp('
367+
];
368+
const FUNC_TAGS = ['S', 'C', 'T', 'Q', 'G', 'N', 'A', 'X'];
369+
const placeholder = (i) => `\u0001${FUNC_TAGS[i]}`;
370+
371+
FUNCS.forEach((name, i) => {
372+
sanitized = sanitized.split(name + '(').join(placeholder(i) + '(');
373+
});
374+
355375
// Implicit multiplication (e.g., 3pi -> 3*pi, 3sin -> 3*sin, (x)(y) -> (x)*(y))
356-
sanitized = sanitized.replace(/(\d)(pi|π|e|x|sin|cos|tan|log|ln|sqrt|abs|exp|\()/g, '$1*$2');
357-
sanitized = sanitized.replace(/(\))(pi|π|e|x|sin|cos|tan|log|ln|sqrt|abs|exp|\(|\d)/g, '$1*$2');
358-
sanitized = sanitized.replace(/(x|pi|π|e)(\d|sin|cos|tan|log|ln|sqrt|abs|exp|\(|x|pi|π|e)/g, '$1*$2');
376+
// Function placeholders are matched as a single unit (control char +
377+
// uppercase letter) so they're never split apart like the old
378+
// literal "sin|cos|...|exp" alternatives could be.
379+
const FUNC_MARKER = '\\u0001[A-Z]';
380+
sanitized = sanitized.replace(new RegExp(`(\\d)(pi|\u03c0|e|x|${FUNC_MARKER}|\\()`, 'g'), '$1*$2');
381+
sanitized = sanitized.replace(new RegExp(`(\\))(pi|\u03c0|e|x|${FUNC_MARKER}|\\(|\\d)`, 'g'), '$1*$2');
382+
sanitized = sanitized.replace(new RegExp(`(x|pi|\u03c0|e)(\\d|${FUNC_MARKER}|\\(|x|pi|\u03c0|e)`, 'g'), '$1*$2');
359383

360384
// Fix JS SyntaxError for unary minus and bind minus to numbers for exponentiation (e.g. -1^2 becomes (-1)^2 = 1)
361385
sanitized = sanitized.replace(/(^|[\(\+\-\*\/\%\^])\s*-([\d\.]+|x|pi|π|e)/g, '$1(-$2)');
@@ -365,19 +389,17 @@ function initCalculator() {
365389
sanitized = sanitized.replace(/(^|[\(\+\-\*\/\%\^])\s*-(?![\d\.]|x|pi|π|e)/g, '$1(-1)*');
366390
sanitized = sanitized.replace(/(^|[\(\+\-\*\/\%\^])\s*-(?![\d\.]|x|pi|π|e)/g, '$1(-1)*');
367391

368-
// Functions
369-
sanitized = sanitized.replace(/sin\(/g, 'Math.sin(');
370-
sanitized = sanitized.replace(/cos\(/g, 'Math.cos(');
371-
sanitized = sanitized.replace(/tan\(/g, 'Math.tan(');
372-
sanitized = sanitized.replace(/sqrt\(/g, 'Math.sqrt(');
373-
sanitized = sanitized.replace(/log\(/g, 'Math.log10(');
374-
sanitized = sanitized.replace(/ln\(/g, 'Math.log(');
375-
sanitized = sanitized.replace(/abs\(/g, 'Math.abs(');
376-
sanitized = sanitized.replace(/exp\(/g, 'Math.exp(');
392+
// Constants -- these now only ever match a standalone "e" or "pi"/"π",
393+
// since every function name is already an opaque placeholder.
377394
sanitized = sanitized.replace(/pi/g, 'Math.PI');
378395
sanitized = sanitized.replace(/π/g, 'Math.PI');
379396
sanitized = sanitized.replace(/e/g, 'Math.E');
380397

398+
// Restore function placeholders to their real Math.* calls.
399+
FUNCS.forEach((name, i) => {
400+
sanitized = sanitized.split(placeholder(i) + '(').join(FUNC_REPLACEMENTS[i]);
401+
});
402+
381403
// Power operator
382404
sanitized = sanitized.replace(/\^/g, '**');
383405

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
// tests-e2e/calculator.spec.js
2+
//
3+
// Regression coverage for GitHub issue #1590:
4+
// "[Bug]: Scientific Calculator breaks when using exp()"
5+
//
6+
// compileMathFunction() replaced function names like "exp(" with
7+
// "Math.exp(" via one regex, and *separately* replaced every literal "e"
8+
// character with "Math.E" via another regex, intended for the Euler's
9+
// number constant. But an earlier "implicit multiplication" step ran
10+
// first and tokenized character-by-character: it matched the "e" at the
11+
// start of "exp(1)", then greedily matched the "x" right after it as a
12+
// second token, splitting "exp(1)" into "e" * "xp(1)" before the
13+
// "exp(" -> "Math.exp(" rule ever got a chance to run. The final constant
14+
// substitution then turned the leftover "e" into "Math.E", producing the
15+
// nonsensical "Math.E*xp(1)" and a ReferenceError, surfaced to the user
16+
// as "Invalid Expression".
17+
//
18+
// The fix protects every function name (sin, cos, tan, sqrt, log, ln,
19+
// abs, exp) behind an opaque placeholder *before* any implicit-
20+
// multiplication or constant-substitution regex runs, so "exp(" can never
21+
// be torn apart into "e" + "xp(" again.
22+
//
23+
// Assumes the Playwright test runner (`@playwright/test`) is used for
24+
// files under tests-e2e/, matching the .spec.js naming convention.
25+
26+
const { test, expect } = require('@playwright/test');
27+
const fs = require('fs');
28+
const path = require('path');
29+
30+
const SCRIPT_PATH = path.join(__dirname, '..', 'js', 'projects', 'calculator.js');
31+
const calculatorSource = fs.readFileSync(SCRIPT_PATH, 'utf-8');
32+
33+
const HARNESS_HTML = `
34+
<!DOCTYPE html>
35+
<html>
36+
<head>
37+
<style>
38+
:root {
39+
--surface-color: #ffffff;
40+
--border-color: #cccccc;
41+
--text-color: #111111;
42+
--text-secondary: #666666;
43+
--primary-color: #333333;
44+
--text: #111111;
45+
}
46+
</style>
47+
</head>
48+
<body>
49+
<div id="app"></div>
50+
<script>${calculatorSource}</script>
51+
</body>
52+
</html>
53+
`;
54+
55+
async function evaluate(page, expression) {
56+
await page.fill('#calcInput', expression);
57+
await page.click('.calc-btn.equals');
58+
// A committed evaluation replaces the input's own value with the
59+
// cleaned numeric result (see evaluateStandard's commit branch), and
60+
// clears the secondary live-preview line.
61+
return page.inputValue('#calcInput');
62+
}
63+
64+
test.describe('Scientific Calculator - exp() parsing (issue #1590)', () => {
65+
test('source protects function names with placeholders before constant substitution', () => {
66+
// Guards directly against the regression: dropping the placeholder
67+
// step (or reordering it after the implicit-multiplication / "e"
68+
// constant regexes) reintroduces the exact bug from issue #1590.
69+
expect(calculatorSource).toMatch(/FUNCS\.forEach/);
70+
expect(calculatorSource).toMatch(/placeholder\(/);
71+
});
72+
73+
test.beforeEach(async ({ page }) => {
74+
await page.setContent(HARNESS_HTML);
75+
await page.evaluate(() => {
76+
document.getElementById('app').innerHTML = getCalculatorHTML();
77+
initCalculator();
78+
});
79+
});
80+
81+
test('exp(1) evaluates to Euler\u2019s number, not "Invalid Expression"', async ({ page }) => {
82+
const value = await evaluate(page, 'exp(1)');
83+
expect(Number(value)).toBeCloseTo(Math.E, 9);
84+
});
85+
86+
test('exp(0) evaluates to 1', async ({ page }) => {
87+
const value = await evaluate(page, 'exp(0)');
88+
expect(Number(value)).toBe(1);
89+
});
90+
91+
test('implicit multiplication before exp still works: 3exp(1)', async ({ page }) => {
92+
const value = await evaluate(page, '3exp(1)');
93+
expect(Number(value)).toBeCloseTo(3 * Math.E, 6);
94+
});
95+
96+
test('nested exp() calls work: exp(exp(1))', async ({ page }) => {
97+
const value = await evaluate(page, 'exp(exp(1))');
98+
expect(Number(value)).toBeCloseTo(Math.exp(Math.E), 6);
99+
});
100+
101+
test('exp() combined with the standalone "e" constant: exp(1)+e', async ({ page }) => {
102+
const value = await evaluate(page, 'exp(1)+e');
103+
expect(Number(value)).toBeCloseTo(Math.E + Math.E, 6);
104+
});
105+
106+
test('the standalone "e" constant still evaluates correctly on its own', async ({ page }) => {
107+
const value = await evaluate(page, 'e^2');
108+
expect(Number(value)).toBeCloseTo(Math.E ** 2, 6);
109+
});
110+
111+
test('other scientific functions are unaffected by the fix', async ({ page }) => {
112+
expect(Number(await evaluate(page, 'sin(0)'))).toBe(0);
113+
expect(Number(await evaluate(page, 'sqrt(4)'))).toBe(2);
114+
expect(Number(await evaluate(page, 'log(100)'))).toBe(2);
115+
expect(Number(await evaluate(page, 'ln(e)'))).toBeCloseTo(1, 9);
116+
});
117+
});

0 commit comments

Comments
 (0)