Repository: source-academy/js-slang (ts)

Issue:
Error message: "Undefined Variable" 
Currently we have:

Line 1: Undefined Variable aasdf

I'm suggesting instead:

Line 1: Name aasdf not declared

(The term "name" is meant to capture constants and variables.)

Pull request:
Change undefined variable message
Resolves #33.

Patch:
diff --git a/doc/source_styleguide.tex b/doc/source_styleguide.tex
index 644a2818..bd330247 100644
--- a/doc/source_styleguide.tex
+++ b/doc/source_styleguide.tex
@@ -2,7 +2,7 @@
 
 \begin{document}
 	%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
-	\docheader{beta release}{Source}{Style Guide}
+	\docheader{Version 1.0}{Source}{Style Guide}
 	%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
 
 This is the style guide for the language Source,
@@ -35,6 +35,17 @@ function make_abs_adder(x) {
 Note the four spaces before \lstinline{return}, and then indentation of the \lstinline{if}
 statement four spaces to the right of the preceding \lstinline{function}.
 
+Four space indent also applies when individual lines get too long and
+need to be broken at convenient places. Example:
+\begin{lstlisting}
+function frac_sum(a, b) {
+    return (a > b) ? 0
+        : 1 / (a * (a + 2))
+            +
+            frac_sum(a + 4, b);
+}
+\end{lstlisting}
+
 \section*{Line Length}
   Lines should be truncated such that they do not require excessive horizontal scrolling.
   As a guide, it should be \emph{no more than 80 characters}.
@@ -104,10 +115,10 @@ const x = 1 + 1;
 const x=1+1;
 
 // good style
-return (x === 0) ? "zero" : "not zero";
+return x === 0 ? "zero" : "not zero";
 
 // bad style
-return (x === 0)?"zero":"not zero";
+return x === 0?"zero":"not zero";
 \end{lstlisting}
 	
 	Do not leave a space between unary operators and the variable involved.
@@ -120,43 +131,68 @@ const negative_x = -x;
 const negative_x = - x;
 \end{lstlisting}
 
-\subsection*{Function Definitions}
+\subsection*{Function Definition Expressions}
 
 Keep the parameters and the body expression of a function definition expression
 in one line, if they are short enough.
 
 If they are lengthy, use
-indentation. The indentation starts just below the first character of the \textit{parameter},
-or the open parenthesis, if there are multiple parameters. If the body expression does not
-fit in one line, use indentation so that the subsequent lines start after the blank character
-after the arrow.
+indentation. The indentation starts four characters after first character of
+the \textit{parameter}, if there is only one,
+or four characters after the open parenthesis,
+if there are multiple parameters. If the body expression does not
+fit in one line, use indentation following the first line of the body
+expression. 
 
 \begin{lstlisting}
 // good style
 function count_buttons(garment) {
     return accumulate((sleaves, total) => sleaves + total,
-                      0, 
-		      map(jacket
-		          => is_checkered(jacket)
-		             ? count_buttons(jacket)
-                             : 1,
-			  garment));
+        0, 
+        map(jacket =>
+            is_checkered(jacket)
+                ? count_buttons(jacket)
+                : 1,
+                garment));
 }
 
-// bad style
+// good style
+function count_buttons(garment) {
+    return accumulate(
+        (sleaves, total) =>
+            delicate_calculation(sleaves + total),
+        0,
+        map(jacket =>
+            is_checkered(jacket)
+                ? count_buttons(jacket)
+                : 1,
+            garment));
+}
+
+// bad style: too much indentation
 function count_buttons(garment) {
-    return accumulate((sleaves, total)
-                      =>
-                      sleaves + total,
+    return accumulate((sleaves, total) =>
+                      delicate_calculation(sleaves + total),
                       0, 
-		      map(jacket
-		          => is_checkered(jacket)
-		          ? count_buttons(jacket)
+		      map(jacket =>
+                          is_checkered(jacket)
+                          ? count_buttons(jacket)
                           : 1,
-			  garment));
+                          garment));
 }
 
-
+// no newline allowed between parameters and =>
+function count_buttons(garment) {
+    return accumulate(
+        (sleaves, total) 
+            => delicate_calculation(sleaves + total),
+        0,
+        map(jacket 
+            => is_checkered(jacket)
+                ? count_buttons(jacket)
+                : 1,
+            garment));
+}
 \end{lstlisting}
 
 
@@ -170,30 +206,31 @@ const aspect_ratio = landscape ? 4 / 3 : 3 / 4;
 
 // bad style
 const aspect_ratio = landscape
-                   ? 4 / 3
-                   : 3 / 4;
+    ? 4 / 3
+    : 3 / 4;
 \end{lstlisting}
 
 If the \textit{consequent-expression} or \textit{alternative-expression} are lengthy, use
-indentation. The indentation starts just below the \textit{predicate}.
+indentation. The indentation is as usual four characters longer
+than the indentation of the previous line.
 
 \begin{lstlisting}
 // good style
 function A(x,y) {
     return y === 0
-           ? 0
-           : x === 0
-             ? 2 * y
-             : y === 1
-               ? 2
-               : A(x - 1, A(x, y - 1));
-
-// bad style
+        ? 0
+        : x === 0
+            ? 2 * y
+            : y === 1
+                ? 2
+                : A(x - 1, A(x, y - 1));
+
+// bad style: line too long
 function A(x,y) {
     return y === 0 ? 0 : x === 0 ? 2 * y : y === 1 ? 2 : A(x - 1, A(x, y - 1));
 }
 
-// bad style
+// bad style: too much indentation
 function A(x,y) {
     return y === 0
                ? 0
@@ -275,12 +312,14 @@ if (x === 1) {
 
   Clean up \emph{all trailing whitespace} before submitting your program.
 
-\section*{Variables}
-\subsection*{Naming}
-When naming variables, use \emph{underscores} to separate words. Examples: \lstinline{my_variable}, \lstinline{x}, \lstinline{rcross_bb}.
+\section*{Names}
+\subsection*{Choice of names}
+When naming constants or variables,
+use \emph{underscores} to separate words.
+Examples: \lstinline{my_variable}, \lstinline{x}, \lstinline{rcross_bb}.
 
 \subsection*{Nesting}
-Do not use the same variable name for nested scope. Examples:
+Do not use the same name for nested scope. Examples:
 \begin{lstlisting}
 // bad program
 const x = 1;
diff --git a/src/interpreter-errors.ts b/src/interpreter-errors.ts
index 49c6b41a..3769a9ca 100644
--- a/src/interpreter-errors.ts
+++ b/src/interpreter-errors.ts
@@ -105,7 +105,7 @@ export class UndefinedVariable implements SourceError {
   }
 
   public explain() {
-    return `Undefined Variable ${this.name}`
+    return `Name ${this.name} not declared`
   }
 
   public elaborate() {


Test patch:
diff --git a/src/__tests__/__snapshots__/interpreter-errors.ts.snap b/src/__tests__/__snapshots__/interpreter-errors.ts.snap
new file mode 100644
index 00000000..1bf6b341
--- /dev/null
+++ b/src/__tests__/__snapshots__/interpreter-errors.ts.snap
@@ -0,0 +1,27 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Undefined variable error is thrown 1`] = `
+Object {
+  "status": "error",
+}
+`;
+
+exports[`Undefined variable error is thrown 2`] = `
+Array [
+  UndefinedVariable {
+    "location": SourceLocation {
+      "end": Position {
+        "column": 16,
+        "line": 2,
+      },
+      "start": Position {
+        "column": 4,
+        "line": 2,
+      },
+    },
+    "name": "im_undefined",
+    "severity": "Error",
+    "type": "Runtime",
+  },
+]
+`;
diff --git a/src/__tests__/interpreter-errors.ts b/src/__tests__/interpreter-errors.ts
new file mode 100644
index 00000000..115bc94a
--- /dev/null
+++ b/src/__tests__/interpreter-errors.ts
@@ -0,0 +1,16 @@
+import { mockContext } from "../mocks/context";
+import { runInContext, parseError } from "../index";
+
+test("Undefined variable error is thrown", () => {
+  const code = `
+    im_undefined;
+  `
+  const context = mockContext()
+  const promise = runInContext(code, context, { scheduler: "preemptive" })
+  return promise.then(obj => {
+    expect(obj).toMatchSnapshot()
+    expect(obj.status).toBe("error")
+    expect(context.errors).toMatchSnapshot()
+    expect(parseError(context.errors)).toBe("Line 2: Name im_undefined not declared")
+  })
+})