[rust][ci] Run cargo check/fmt

Formats the codebase, fixes some clippy lints, and updates CI to check that code 
is formatted.

compiler/.github/workflows/rust.yml

@@ -17,15 +17,53 @@ on:
 
 env:
   CARGO_TERM_COLOR: always
+  RUSTFLAGS: -Dwarnings
 
 jobs:
   test:
-    runs-on: ubuntu-latest
+    name: Rust Test (${{ matrix.target.os }})
+    strategy:
+      matrix:
+        target:
+          - target: ubuntu-latest
+            os: ubuntu-latest
+          # TODO: run on more platforms
+          # - target: macos-latest
+          #   os: macos-latest
+          # - target: windows-latest
+          #   os: windows-latest
+    runs-on: ${{ matrix.target.os }}
     steps:
       - uses: actions/checkout@v3
-      - name: Build
-        run: cargo build --verbose
+      - name: cargo test
         working-directory: forget
-      - name: Run tests
-        run: cargo test --verbose
+        run: cargo test --manifest-path=Cargo.toml --locked ${{ matrix.target.features && '--features' }} ${{ matrix.target.features }}
+
+  lint:
+    name: Rust Lint
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v2
+      - uses: actions-rs/toolchain@v1
+        with:
+          # NOTE: use `rustup run <toolchain> <command>` in commands below
+          # with this exact same toolchain value
+          toolchain: nightly-2023-08-01
+          override: true
+          components: rustfmt, clippy
+      - name: rustfmt
+        run: grep -r --include "*.rs" --files-without-match "@generated" crates | xargs rustup run nightly-2023-08-01 rustfmt --check --config="skip_children=true"
+        working-directory: ./forget/
+      # - name: cargo clippy
+      #   working-directory: forget
+      #   run: rustup run nightly-2023-08-01 cargo clippy -- -Dclippy::correctness
+
+  build:
+    name: Rust Build
+    runs-on: ubuntu-latest
+    # TODO: build on more platforms, deploy, etc
+    steps:
+      - uses: actions/checkout@v3
+      - name: cargo build
         working-directory: forget
+        run: cargo build --release

compiler/forget/crates/forget_build_hir/src/build.rs

@@ -374,13 +374,13 @@ fn lower_expression(
         Expression::AssignmentExpression(expr) => match expr.operator {
             forget_estree::AssignmentOperator::Equals => {
                 let right = lower_expression(env, builder, &expr.right)?;
-                return Ok(lower_assignment(
+                return lower_assignment(
                     env,
                     builder,
                     InstructionKind::Reassign,
                     &expr.left,
                     right,
-                )?);
+                );
             }
             _ => todo!("lower assignment expr {:#?}", expr),
         },
@@ -418,7 +418,7 @@ fn lower_expression(
                 return Err(Diagnostic::todo("Support method calls", expr.range));
             }
 
-            let callee = lower_expression(env, builder, &callee_expr)?;
+            let callee = lower_expression(env, builder, callee_expr)?;
             let arguments = lower_arguments(env, builder, &expr.arguments)?;
             InstructionValue::Call(forget_hir::Call { callee, arguments })
         }
@@ -568,7 +568,7 @@ fn lower_assignment_pattern(
                     None => items.push(ArrayDestructureItem::Hole),
                     Some(Pattern::Identifier(element)) => {
                         let identifier =
-                            lower_identifier_for_assignment(env, builder, kind, &element)?;
+                            lower_identifier_for_assignment(env, builder, kind, element)?;
                         items.push(ArrayDestructureItem::Value(identifier));
                     }
                     Some(Pattern::RestElement(element)) => {

compiler/forget/crates/forget_diagnostics/src/diagnostic.rs

@@ -2,7 +2,7 @@ use std::error::Error;
 use std::fmt::{Debug, Display, Write};
 
 use forget_estree::SourceRange;
-use miette::{ByteOffset, SourceSpan};
+use miette::SourceSpan;
 use static_assertions::assert_impl_all;
 use thiserror::Error;
 
@@ -189,7 +189,7 @@ impl Diagnostic {
 
 impl Display for Diagnostic {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.0.message.to_string())
+        write!(f, "{}", self.0.message)
     }
 }
 
@@ -272,7 +272,7 @@ impl From<Diagnostic> for Diagnostics {
 
 fn source_span_from_range(range: SourceRange) -> SourceSpan {
     SourceSpan::new(
-        ByteOffset::from(range.start as usize).into(),
-        ByteOffset::from((u32::from(range.end) - range.start) as usize).into(),
+        (range.start as usize).into(),
+        ((u32::from(range.end) - range.start) as usize).into(),
     )
 }

compiler/forget/crates/forget_estree/src/generated.rs

@@ -3,6 +3,7 @@
 #![allow(dead_code)]
 #![allow(unused_variables)]
 #![allow(non_snake_case)]
+#![allow(clippy::enum_variant_names)]
 use std::num::NonZeroU32;
 use serde::ser::{Serializer, SerializeMap};
 use serde::{Serialize, Deserialize};

compiler/forget/crates/forget_estree/src/js_value.rs

@@ -15,7 +15,7 @@ impl JsValue {
         match &self {
             JsValue::Boolean(value) => *value,
             JsValue::Number(value) => value.is_truthy(),
-            JsValue::String(value) => value.len() != 0,
+            JsValue::String(value) => !value.is_empty(),
             JsValue::Null => false,
             JsValue::Undefined => false,
         }
@@ -102,7 +102,7 @@ impl<'de> Deserialize<'de> for JsValue {
 
             #[inline]
             fn visit_i64<E>(self, value: i64) -> Result<JsValue, E> {
-                if value >= MIN_SAFE_INT && value <= MAX_SAFE_INT {
+                if (MIN_SAFE_INT..=MAX_SAFE_INT).contains(&value) {
                     Ok(JsValue::Number((value as f64).into()))
                 } else {
                     panic!("Invalid number")
@@ -212,11 +212,7 @@ impl Number {
 
     pub fn is_truthy(self) -> bool {
         let value = f64::from(self);
-        if self.0 == f64::NAN.to_bits() || value == 0.0 || value == -0.0 {
-            false
-        } else {
-            true
-        }
+        !(self.0 == f64::NAN.to_bits() || value == 0.0 || value == -0.0)
     }
 }
 

compiler/forget/crates/forget_estree/src/lib.rs

@@ -15,7 +15,6 @@ pub use visit::*;
 #[cfg(test)]
 mod tests {
     use insta::{assert_snapshot, glob};
-    use serde_json;
 
     use super::*;
 

compiler/forget/crates/forget_estree/src/visit.rs

@@ -14,14 +14,14 @@ use crate::{
 pub trait Visitor_DEPRECATED<'ast> {
     fn visit_lvalue<F>(&mut self, f: F)
     where
-        F: FnOnce(&mut Self) -> (),
+        F: FnOnce(&mut Self),
     {
         f(self);
     }
 
     fn visit_rvalue<F>(&mut self, f: F)
     where
-        F: FnOnce(&mut Self) -> (),
+        F: FnOnce(&mut Self),
     {
         f(self);
     }

compiler/forget/crates/forget_estree_codegen/src/codegen.rs

@@ -92,6 +92,7 @@ impl Grammar {
             #![allow(dead_code)]
             #![allow(unused_variables)]
             #![allow(non_snake_case)]
+            #![allow(clippy::enum_variant_names)]
 
             use std::num::NonZeroU32;
             use serde::ser::{Serializer, SerializeMap};
@@ -137,9 +138,9 @@ impl Grammar {
         quote! {
             #![allow(dead_code)]
             #![allow(unused_variables)]
+            #![allow(clippy::enum_variant_names)]
 
             use forget_estree::*;
-            use hermes;
             use hermes::parser::{NodePtr, NodeKind, NodeLabel };
             use hermes::utf::{utf8_with_surrogates_to_string};
             use crate::generated_extension::*;
@@ -335,7 +336,7 @@ impl Node {
             .filter_map(|(name, field)| {
                 let (type_name_str, type_kind) = parse_type(&field.type_).unwrap();
                 if (!grammar.objects.contains_key(&type_name_str)
-                    || grammar.objects.get(&type_name_str).unwrap().visitor == false)
+                    || !grammar.objects.get(&type_name_str).unwrap().visitor)
                     && !grammar.nodes.contains_key(&type_name_str)
                     && !grammar.enums.contains_key(&type_name_str)
                 {
@@ -926,7 +927,7 @@ fn parse_type(type_: &str) -> Result<(String, TypeKind), String> {
         current = &current[7..current.len() - 1];
         is_option = true;
     }
-    if current.contains("<") {
+    if current.contains('<') {
         Err(format!(
             "Unsupported type `{current}` expected named type (`Identifier`), optional type (`Option<Identifier>`), list type (`Vec<Identifier>`), or optional list (`Vec<Option<Identifier>>`)"
         ))

compiler/forget/crates/forget_hermes_parser/src/generated.rs

@@ -2,8 +2,8 @@
 #![cfg_attr(rustfmt, rustfmt_skip)]
 #![allow(dead_code)]
 #![allow(unused_variables)]
+#![allow(clippy::enum_variant_names)]
 use forget_estree::*;
-use hermes;
 use hermes::parser::{NodePtr, NodeKind, NodeLabel};
 use hermes::utf::utf8_with_surrogates_to_string;
 use crate::generated_extension::*;

compiler/forget/crates/forget_hermes_parser/src/generated_extension.rs

@@ -19,8 +19,8 @@ use hermes::parser::{
     hermes_get_FunctionExpression_generator, hermes_get_FunctionExpression_id,
     hermes_get_FunctionExpression_params, hermes_get_Property_computed, hermes_get_Property_key,
     hermes_get_Property_kind, hermes_get_Property_method, hermes_get_Property_shorthand,
-    hermes_get_Property_value, NodeKind, NodeLabel, NodeLabelOpt, NodeListRef,
-    NodePtr, NodePtrOpt, NodeString, NodeStringOpt, SMRange,
+    hermes_get_Property_value, NodeKind, NodeLabel, NodeLabelOpt, NodeListRef, NodePtr, NodePtrOpt,
+    NodeString, NodeStringOpt, SMRange,
 };
 use hermes::utf::utf8_with_surrogates_to_string;
 use juno_support::NullTerminatedBuf;
@@ -31,7 +31,7 @@ pub struct Context {
 
 impl Context {
     pub fn new(parser: &NullTerminatedBuf) -> Self {
-        let start: usize = unsafe { std::mem::transmute(parser.as_ptr()) };
+        let start: usize = unsafe { parser.as_ptr() as usize };
         Self { start }
     }
 }
@@ -43,11 +43,11 @@ pub trait FromHermesLabel {
     fn convert(cx: &mut Context, label: NodeLabel) -> Self;
 }
 
-pub fn convert_option<F, T>(node: NodePtrOpt, mut f: F) -> Option<T>
+pub fn convert_option<F, T>(node: NodePtrOpt, f: F) -> Option<T>
 where
     F: FnMut(NodePtr) -> T,
 {
-    node.as_node_ptr().map(|node| f(node))
+    node.as_node_ptr().map(f)
 }
 
 pub fn convert_vec<F, T>(node: NodeListRef, mut f: F) -> Vec<T>
@@ -75,9 +75,9 @@ where
 
 pub fn convert_range(cx: &Context, node: NodePtr) -> SourceRange {
     let range = node.as_ref().source_range;
-    let absolute_start: usize = unsafe { std::mem::transmute(range.start.as_ptr()) };
+    let absolute_start: usize = range.start.as_ptr() as usize;
     let start = absolute_start - cx.start;
-    let absolute_end: usize = unsafe { std::mem::transmute(range.end.as_ptr()) };
+    let absolute_end: usize = range.end.as_ptr() as usize;
     let end = absolute_end - cx.start;
     SourceRange {
         start: start as u32,

compiler/forget/crates/forget_hermes_parser/src/lib.rs

@@ -23,7 +23,7 @@ pub fn parse(source: &str, _file: &str) -> Result<Program, Vec<Diagnostic>> {
     if result.has_errors() {
         let error_messages = result.messages();
         return Err(error_messages
-            .into_iter()
+            .iter()
             .map(|diag| {
                 let message = utf8_with_surrogates_to_string(diag.message.as_slice()).unwrap();
                 Diagnostic::invalid_syntax(message, None)

compiler/forget/crates/forget_hermes_parser/tests/parser_test.rs

@@ -3,7 +3,6 @@ use std::env;
 use forget_estree::SourceType;
 use forget_hermes_parser::parse;
 use insta::{assert_snapshot, glob};
-use serde_json;
 
 #[test]
 fn fixtures() {

compiler/forget/crates/forget_hir/src/function.rs

@@ -32,7 +32,7 @@ pub struct HIR {
 }
 
 impl HIR {
-    pub fn inline(&mut self, other: FunctionExpression) -> () {
+    pub fn inline(&mut self, other: FunctionExpression) {
         let offset = self.instructions.len();
         for instr in other.lowered_function.body.instructions.into_iter() {
             self.instructions.push(instr);
@@ -197,7 +197,7 @@ impl<'blocks> BlockRewriter<'blocks> {
         }
     }
 
-    pub fn each_block<F>(&mut self, mut f: F) -> ()
+    pub fn each_block<F>(&mut self, mut f: F)
     where
         F: FnMut(Box<BasicBlock>, &mut Self) -> BlockRewriterAction,
     {

compiler/forget/crates/forget_hir/src/instruction.rs

@@ -14,9 +14,9 @@ pub struct Instruction {
 }
 
 impl Instruction {
-    pub fn each_lvalue<F>(&mut self, mut f: F) -> ()
+    pub fn each_lvalue<F>(&mut self, mut f: F)
     where
-        F: FnMut(&mut IdentifierOperand) -> (),
+        F: FnMut(&mut IdentifierOperand),
     {
         match &mut self.value {
             InstructionValue::DeclareContext(instr) => {
@@ -75,9 +75,9 @@ impl Instruction {
         Ok(())
     }
 
-    pub fn each_rvalue<F>(&mut self, mut f: F) -> ()
+    pub fn each_rvalue<F>(&mut self, mut f: F)
     where
-        F: FnMut(&mut IdentifierOperand) -> (),
+        F: FnMut(&mut IdentifierOperand),
     {
         match &mut self.value {
             InstructionValue::Array(value) => {
@@ -303,9 +303,9 @@ impl DestructurePattern {
         }
         Ok(())
     }
-    pub fn each_operand<F>(&mut self, f: &mut F) -> ()
+    pub fn each_operand<F>(&mut self, f: &mut F)
     where
-        F: FnMut(&mut IdentifierOperand) -> (),
+        F: FnMut(&mut IdentifierOperand),
     {
         match self {
             Self::Array(elements) => {

compiler/forget/crates/forget_hir/src/terminal.rs

@@ -30,7 +30,7 @@ pub enum TerminalValue {
 }
 
 impl TerminalValue {
-    pub fn map_optional_fallthroughs<F>(&mut self, f: F) -> ()
+    pub fn map_optional_fallthroughs<F>(&mut self, f: F)
     where
         F: Fn(BlockId) -> Option<BlockId>,
     {
@@ -85,9 +85,9 @@ impl TerminalValue {
         }
     }
 
-    pub fn each_operand<F>(&mut self, mut f: F) -> ()
+    pub fn each_operand<F>(&mut self, mut f: F)
     where
-        F: FnMut(&mut IdentifierOperand) -> (),
+        F: FnMut(&mut IdentifierOperand),
     {
         match self {
             TerminalValue::Branch(terminal) => f(&mut terminal.test),

compiler/forget/crates/forget_optimization/src/constant_propagation.rs

@@ -220,15 +220,11 @@ fn apply_binary_operator(
             _ => None,
         },
         (left, right) => match operator {
-            BinaryOperator::Equals => left
-                .loosely_equals(&right)
-                .map(|value| JsValue::Boolean(value)),
-            BinaryOperator::NotEquals => left
-                .not_loosely_equals(&right)
-                .map(|value| JsValue::Boolean(value)),
-            BinaryOperator::StrictEquals => Some(JsValue::Boolean(left.strictly_equals(&right))),
+            BinaryOperator::Equals => left.loosely_equals(right).map(JsValue::Boolean),
+            BinaryOperator::NotEquals => left.not_loosely_equals(right).map(JsValue::Boolean),
+            BinaryOperator::StrictEquals => Some(JsValue::Boolean(left.strictly_equals(right))),
             BinaryOperator::NotStrictEquals => {
-                Some(JsValue::Boolean(left.not_strictly_equals(&right)))
+                Some(JsValue::Boolean(left.not_strictly_equals(right)))
             }
             _ => None,
         },

compiler/forget/crates/forget_semantic_analysis/src/analyzer.rs

@@ -79,7 +79,7 @@ impl Analyzer {
 
     fn enter_label<F>(&mut self, id: LabelId, mut f: F)
     where
-        F: FnMut(&mut Self) -> (),
+        F: FnMut(&mut Self),
     {
         self.labels.push(id);
         f(self);
@@ -127,7 +127,7 @@ impl Analyzer {
 
     fn enter<F>(&mut self, kind: ScopeKind, mut f: F) -> ScopeId
     where
-        F: FnMut(&mut Self) -> (),
+        F: FnMut(&mut Self),
     {
         let scope = self.enter_scope(kind);
         f(self);

compiler/forget/crates/forget_semantic_analysis/src/scope_view.rs

@@ -13,7 +13,7 @@ pub struct ScopeManagerView<'m> {
 impl<'m> ScopeManagerView<'m> {
     pub fn root(&self) -> ScopeView<'m> {
         ScopeView {
-            manager: &self.manager,
+            manager: self.manager,
             scope: self.manager.scope(self.manager.root_id()),
         }
     }
@@ -50,7 +50,7 @@ impl<'m> ScopeView<'m> {
         self.scope.parent.map(|id| {
             let scope = self.manager.scope(id);
             ScopeView {
-                manager: &self.manager,
+                manager: self.manager,
                 scope,
             }
         })
@@ -64,7 +64,7 @@ impl<'m> ScopeView<'m> {
             .map(|id| {
                 let declaration = self.manager.declaration(id);
                 DeclarationView {
-                    manager: &self.manager,
+                    manager: self.manager,
                     declaration,
                 }
             })
@@ -79,7 +79,7 @@ impl<'m> ScopeView<'m> {
             .map(|id| {
                 let reference = self.manager.reference(id);
                 ReferenceView {
-                    manager: &self.manager,
+                    manager: self.manager,
                     reference,
                 }
             })
@@ -94,7 +94,7 @@ impl<'m> ScopeView<'m> {
             .map(|id| {
                 let scope = self.manager.scope(id);
                 ScopeView {
-                    manager: &self.manager,
+                    manager: self.manager,
                     scope,
                 }
             })
@@ -117,7 +117,7 @@ impl<'m> std::fmt::Debug for ScopeView<'m> {
                 (
                     name.clone(),
                     DeclarationView {
-                        manager: &self.manager,
+                        manager: self.manager,
                         declaration: self.manager.declaration(*declaration),
                     },
                 )
@@ -128,7 +128,7 @@ impl<'m> std::fmt::Debug for ScopeView<'m> {
             .references
             .iter()
             .map(|reference| ReferenceView {
-                manager: &self.manager,
+                manager: self.manager,
                 reference: self.manager.reference(*reference),
             })
             .collect();
@@ -137,7 +137,7 @@ impl<'m> std::fmt::Debug for ScopeView<'m> {
             .children
             .iter()
             .map(|child| ScopeView {
-                manager: &self.manager,
+                manager: self.manager,
                 scope: self.manager.scope(*child),
             })
             .collect();
@@ -190,7 +190,7 @@ impl<'m> DeclarationView<'m> {
     pub fn scope(&self) -> ScopeView<'m> {
         let scope = self.manager.scope(self.declaration.scope);
         ScopeView {
-            manager: &self.manager,
+            manager: self.manager,
             scope,
         }
     }
@@ -224,7 +224,7 @@ impl<'m> ReferenceView<'m> {
     pub fn scope(&self) -> ScopeView<'m> {
         let scope = self.manager.scope(self.reference.scope);
         ScopeView {
-            manager: &self.manager,
+            manager: self.manager,
             scope,
         }
     }
@@ -232,7 +232,7 @@ impl<'m> ReferenceView<'m> {
     pub fn declaration(&self) -> DeclarationView<'m> {
         let declaration = self.manager.declaration(self.reference.declaration);
         DeclarationView {
-            manager: &self.manager,
+            manager: self.manager,
             declaration,
         }
     }

compiler/forget/crates/forget_ssa/src/enter.rs

@@ -38,9 +38,9 @@ pub fn enter_ssa_impl(
     Ok(())
 }
 
-fn visit_instructions<'e>(
+fn visit_instructions(
     env: &Environment,
-    builder: &mut Builder<'e>,
+    builder: &mut Builder<'_>,
     hir: &mut HIR,
 ) -> Result<(), Diagnostic> {
     let instructions = &mut hir.instructions;
@@ -175,7 +175,7 @@ impl<'e> Builder<'e> {
         Ok(())
     }
 
-    fn visit_param(&mut self, param: &mut IdentifierOperand) -> () {
+    fn visit_param(&mut self, param: &mut IdentifierOperand) {
         let old_identifier = &param.identifier;
         let new_identifier = self.make_identifier(old_identifier);
         let state = self.states.get_mut(&self.current).unwrap();
@@ -183,7 +183,7 @@ impl<'e> Builder<'e> {
         param.identifier = new_identifier;
     }
 
-    fn visit_load(&mut self, local: &mut IdentifierOperand) -> () {
+    fn visit_load(&mut self, local: &mut IdentifierOperand) {
         let new_identifier = self.get_id_at(self.current, &local.identifier);
         local.identifier = new_identifier;
     }
@@ -262,7 +262,7 @@ impl<'e> Builder<'e> {
         }
     }
 
-    fn fix_incomplete_phis(&mut self, block_id: BlockId) -> () {
+    fn fix_incomplete_phis(&mut self, block_id: BlockId) {
         let state = self.states.get_mut(&block_id).unwrap();
         let incomplete_phis = std::mem::take(&mut state.incomplete_phis);
         for phi in incomplete_phis {