Add another monstrosity of a switch statement for expressions

I guess...now I just have to...implement Lintable for all these
different types of expressions...yippee...

Cargo.toml

@@ -68,7 +68,7 @@ twox-hash = "1.5.0"
 url = "2.1.1"
 uuid = { version = "0.8", features = ["v4"] }
 wasmparser = "0.63.0"
-wast = "24.0.0"
+wast = "25.0.0"
 which = "4.0.2"
 ws = "0.9.1"
 

src/build/check/js/linter/expressions.rs

@@ -0,0 +1,87 @@
+use super::{AstNodeLinterArgs, Lintable};
+use swc_ecma_ast::Expr;
+
+/// [Expressions](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Expressions_and_Operators#Expressions)
+/// are the things we actually care about linting. From MDN:
+/// > An *expression* is any valid unit of code that resolves to a value
+impl<'a> Lintable<AstNodeLinterArgs<'a>> for Expr {
+    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
+        // I would like to reiterate, MDN is doing it like nobody else. Or, was doing it, I suppose.
+        match self {
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/this
+            Expr::This(_) => Ok(()),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array
+            Expr::Array(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Object_initializer
+            Expr::Object(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/function
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/function*
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/async_function
+            Expr::Fn(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#Unary_operators
+            Expr::Unary(_) => Ok(()),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#Increment_and_decrement
+            Expr::Update(_) => Ok(()),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#Binary_bitwise_operators
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#Binary_logical_operators
+            Expr::Bin(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators#Assignment_operators
+            Expr::Assign(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Property_accessors
+            Expr::Member(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Conditional_Operator
+            Expr::Cond(expression) => expression.lint(args),
+            // https://docs.onux.com/en-US/Developers/JavaScript-PP/Language/Reference/Expressions/function-call
+            Expr::Call(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/new
+            Expr::New(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Comma_Operator
+            Expr::Seq(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Glossary/Identifier
+            Expr::Ident(_) => Ok(()),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp
+            // ...and other literals which don't need linting
+            Expr::Lit(_) => Ok(()),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals
+            Expr::Tpl(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Template_literals#Tagged_templates
+            Expr::TaggedTpl(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions
+            Expr::Arrow(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/class
+            Expr::Class(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/yield
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/yield*
+            Expr::Yield(expression) => expression.lint(args),
+            // As far as I can tell, this is just... `new.target` ...
+            // https://www.ecma-international.org/ecma-262/6.0/#sec-meta-properties
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/new.target
+            Expr::MetaProp(_) => Ok(()),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/await
+            Expr::Await(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Grouping
+            Expr::Paren(expression) => expression.lint(args),
+            Expr::JSXMember(_) => Err(failure::err_msg("JSX is not allowed in workers!")),
+            Expr::JSXNamespacedName(_) => Err(failure::err_msg("JSX is not allowed in workers!")),
+            Expr::JSXEmpty(_) => Err(failure::err_msg("JSX is not allowed in workers!")),
+            Expr::JSXElement(_) => Err(failure::err_msg("JSX is not allowed in workers!")),
+            Expr::JSXFragment(_) => Err(failure::err_msg("JSX is not allowed in workers!")),
+            Expr::TsTypeAssertion(_) => {
+                Err(failure::err_msg("Typescript is not allowed in workers!"))
+            }
+            Expr::TsConstAssertion(_) => {
+                Err(failure::err_msg("Typescript is not allowed in workers!"))
+            }
+            Expr::TsNonNull(_) => Err(failure::err_msg("Typescript is not allowed in workers!")),
+            Expr::TsTypeCast(_) => Err(failure::err_msg("Typescript is not allowed in workers!")),
+            Expr::TsAs(_) => Err(failure::err_msg("Typescript is not allowed in workers!")),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Classes/Private_class_fields
+            Expr::PrivateName(expression) => expression.lint(args),
+            // https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining
+            Expr::OptChain(expression) => expression.lint(args),
+            // TODO: we need to define a custom error type that's usable by match_to_source_map or
+            // whatever it's called and throw that here instead of just failure::Error
+            Expr::Invalid(_) => Err(failure::err_msg("Failed to parse expression!")),
+        }
+    }
+}

src/build/check/js/linter/misc.rs

@@ -0,0 +1,31 @@
+use super::{AstNodeLinterArgs, Lintable};
+use swc_ecma_ast::{Decl, Pat, VarDecl, VarDeclOrPat};
+
+// other AstNodes that aren't expressions or statements
+
+impl<'a> Lintable<AstNodeLinterArgs<'a>> for Decl {
+    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
+        todo!()
+    }
+}
+
+impl<'a> Lintable<AstNodeLinterArgs<'a>> for Pat {
+    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
+        todo!()
+    }
+}
+
+impl<'a> Lintable<AstNodeLinterArgs<'a>> for VarDecl {
+    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
+        todo!()
+    }
+}
+
+impl<'a> Lintable<AstNodeLinterArgs<'a>> for VarDeclOrPat {
+    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
+        match self {
+            VarDeclOrPat::VarDecl(declaration) => declaration.lint(args),
+            VarDeclOrPat::Pat(pattern) => pattern.lint(args),
+        }
+    }
+}

src/build/check/js/linter/mod.rs

@@ -0,0 +1,46 @@
+use sourcemap::SourceMap;
+use swc_ecma_ast::Script;
+
+use super::{ExpressionList, Lintable};
+
+mod expressions;
+mod misc;
+mod statements;
+
+// the difference between the args for linting a Script and linting an AstNode
+// is that the script doesn't need to know whether or not it's in the request context,
+// because it's always *not* in the request context. It does, however, take an optional
+// source map that can be used to map errors to the original source to provide more
+// helpful error messages to developers
+type ScriptLinterArgs<'a> = (Option<&'a SourceMap>, ExpressionList, ExpressionList);
+type AstNodeLinterArgs<'a> = (bool, &'a ExpressionList, &'a ExpressionList);
+
+impl<'a> Lintable<ScriptLinterArgs<'a>> for Script {
+    fn lint(
+        &self,
+        (source_map, unavailable, available_in_request_context): ScriptLinterArgs,
+    ) -> Result<(), failure::Error> {
+        if let Err(error) = self
+            .body
+            .lint((false, &unavailable, &available_in_request_context))
+        {
+            Err(match source_map {
+                Some(map) => match_error_to_source_map(error, map)?,
+                None => error,
+            })
+        } else {
+            Ok(())
+        }
+    }
+}
+
+// TODO: it would be cool to have line numbers in the errors
+// and i don't think it would be like extremely hard to do,
+// since every statement has its own associated byte position.
+// But that's a nice-to-have for sure
+fn match_error_to_source_map(
+    error: failure::Error,
+    source_map: &SourceMap,
+) -> Result<failure::Error, failure::Error> {
+    Ok(failure::format_err!("Thanks for providing us with a source map! Soon hopefully we will be able to tell you what part of your original source code is bad. Unfortunately, for now, all we can say is\n{}", error))
+}

src/build/check/js/linter.rs → src/build/check/js/linter/statements.rs

@@ -1,58 +1,20 @@
-use sourcemap::SourceMap;
 use swc_ecma_ast::{
-    BlockStmt, Decl, DoWhileStmt, Expr, ExprStmt, ForInStmt, ForOfStmt, ForStmt, IfStmt,
-    LabeledStmt, Pat, ReturnStmt, Script, Stmt, SwitchStmt, ThrowStmt, TryStmt, VarDecl,
-    VarDeclOrExpr, VarDeclOrPat, WhileStmt, WithStmt,
+    BlockStmt, DoWhileStmt, ExprStmt, ForInStmt, ForOfStmt, ForStmt, IfStmt, LabeledStmt,
+    ReturnStmt, Stmt, SwitchStmt, ThrowStmt, TryStmt, VarDeclOrExpr, WhileStmt, WithStmt,
 };
 
-use super::{ExpressionList, Lintable};
-
-// the difference between the args for linting a Script and linting an AstNode
-// is that the script doesn't need to know whether or not it's in the request context,
-// because it's always *not* in the request context. It does, however, take an optional
-// source map that can be used to map errors to the original source to provide more
-// helpful error messages to developers
-type ScriptLinterArgs<'a> = (Option<&'a SourceMap>, ExpressionList, ExpressionList);
-type AstNodeLinterArgs<'a> = (bool, &'a ExpressionList, &'a ExpressionList);
-
-impl<'a> Lintable<ScriptLinterArgs<'a>> for Script {
-    fn lint(
-        &self,
-        (source_map, unavailable, available_in_request_context): ScriptLinterArgs,
-    ) -> Result<(), failure::Error> {
-        if let Err(error) = self
-            .body
-            .lint((false, &unavailable, &available_in_request_context))
-        {
-            Err(match source_map {
-                Some(map) => match_error_to_source_map(error, map)?,
-                None => error,
-            })
-        } else {
-            Ok(())
-        }
-    }
-}
-
-// TODO: it would be cool to have line numbers in the errors
-// and i don't think it would be like extremely hard to do,
-// since every statement has its own associated byte position.
-// But that's a nice-to-have for sure
-fn match_error_to_source_map(
-    error: failure::Error,
-    source_map: &SourceMap,
-) -> Result<failure::Error, failure::Error> {
-    Ok(failure::format_err!("Thanks for providing us with a source map! Soon hopefully we will be able to tell you what part of your original source code is bad. Unfortunately, for now, all we can say is\n{}", error))
-}
+use super::{AstNodeLinterArgs, Lintable};
 
 /// By implementing Lintable for Vec<Stmt>, we can call `ast.lint(args)`
 /// at the top level and recurse through the whole AST
 ///
-/// Note: Ideally, the type signature would actually be more general,
-/// `impl<'a, T> Lintable<AstNodeLinterArgs<'a>> for T where T: Iterator<Item = dyn Lintable<AstNodeLinterArgs<'a>>>,`
+/// Note: Ideally, the type signature would actually be more general, like
+/// `impl<'a, T> Lintable<AstNodeLinterArgs<'a>> for T where T: Iterator<Item = dyn Lintable<AstNodeLinterArgs<'a>>>`,
 /// but rustc is not happy about us implementing this when swc might potentially
 /// implement Iterator for e.g. Stmt. Then we'd have conflicting implementations
 /// of Lintable for any struct that also implemented Iterator.
+/// For practical purposes though, this isn't a problem, as swc just uses Vec
+/// for all groups of AstNodes
 impl<'a> Lintable<AstNodeLinterArgs<'a>> for Vec<Stmt> {
     fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
         // this would be cool if it was par_iter...rayon when?
@@ -285,6 +247,17 @@ impl<'a> Lintable<AstNodeLinterArgs<'a>> for ForStmt {
 }
 
 /// [For...in statements](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...in)
+/// have three parts that need linting -- the "left" expression, "right" expression, and the body of the loop.
+/// It's easier to explain with an example.
+///
+/// ```ignore
+/// for (const e in arr) {
+///     // do stuff
+/// }
+///
+/// * `e` is the left expression
+/// * `arr` is the right expression
+/// * `// do stuff` is the body of the loop
 impl<'a> Lintable<AstNodeLinterArgs<'a>> for ForInStmt {
     fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
         self.left.lint(args)?;
@@ -303,41 +276,11 @@ impl<'a> Lintable<AstNodeLinterArgs<'a>> for ForOfStmt {
     }
 }
 
+/// As far as I can tell, the ExprStmt struct is for statements that are just expressions,
+/// like if you made a `fetch()` call without assigning the result to anything. These are
+/// easy to lint because you just have to lint the expression.
 impl<'a> Lintable<AstNodeLinterArgs<'a>> for ExprStmt {
     fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
         self.expr.lint(args)
     }
 }
-
-impl<'a> Lintable<AstNodeLinterArgs<'a>> for Expr {
-    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
-        todo!()
-    }
-}
-
-impl<'a> Lintable<AstNodeLinterArgs<'a>> for Decl {
-    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
-        todo!()
-    }
-}
-
-impl<'a> Lintable<AstNodeLinterArgs<'a>> for Pat {
-    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
-        todo!()
-    }
-}
-
-impl<'a> Lintable<AstNodeLinterArgs<'a>> for VarDecl {
-    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
-        todo!()
-    }
-}
-
-impl<'a> Lintable<AstNodeLinterArgs<'a>> for VarDeclOrPat {
-    fn lint(&self, args: AstNodeLinterArgs) -> Result<(), failure::Error> {
-        match self {
-            VarDeclOrPat::VarDecl(declaration) => declaration.lint(args),
-            VarDeclOrPat::Pat(pattern) => pattern.lint(args),
-        }
-    }
-}