Clippy Lint Workflow (FuelLabs#477)

* draft of clippy lint workflow * testing yml * Update .github/workflows/clippy_check.yml remove redundant keys Co-authored-by: John Adler <adlerjohn@users.noreply.github.com> * Update .github/workflows/clippy_check.yml `core-lang` changed to `sway-core` Co-authored-by: John Adler <adlerjohn@users.noreply.github.com> * Update clippy_check.yml Added `with:` to complete syntax * Token Required Testing if the generic github token is enough for Clippy Check to run * specify args * fix selector debug binary * forc lint & update clippy-workflow * formatting * update args * Requested changes Removed `path` Co-authored-by: John Adler <adlerjohn@users.noreply.github.com> Co-authored-by: Alex Hansen <alex@alex-hansen.com>
mitchmindtree · Jan 1, 2022 · 1ba79ac · 1ba79ac
1 parent 66b0fa1
commit 1ba79ac
Show file tree

Hide file tree

Showing 14 changed files with 91 additions and 81 deletions.
diff --git a/.github/workflows/clippy_check.yml b/.github/workflows/clippy_check.yml
@@ -0,0 +1,18 @@
+name: Clippy Check
+
+on: [ pull_request ]
+
+jobs:
+  clippy_check:
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v2
+
+      - run: rustup component add clippy
+
+      - name: rust-clippy-check
+        uses: actions-rs/clippy-check@v1.0.7
+        with:
+          token: ${{ github.token }}
+          args: --all-features
diff --git a/forc/src/cli/commands/lsp.rs b/forc/src/cli/commands/lsp.rs
@@ -5,5 +5,6 @@ use sway_server::start;
 pub(crate) struct Command {}
 
 pub(crate) async fn exec(_command: Command) -> Result<(), String> {
-    Ok(start().await)
+    start().await;
+    Ok(())
 }
diff --git a/forc/src/cli/commands/test.rs b/forc/src/cli/commands/test.rs
@@ -20,9 +20,8 @@ pub(crate) struct Command {
 pub(crate) fn exec(command: Command) -> Result<(), String> {
     // Cargo args setup
     let mut args: Vec<String> = vec!["test".into()];
-    match command.test_name.to_owned() {
-        Some(name) => args.push(name.clone()),
-        None => {}
+    if let Some(name) = command.test_name {
+        args.push(name)
     };
     args.push("--color".into());
     args.push("always".into());

diff --git a/forc/src/ops/forc_abi_json.rs b/forc/src/ops/forc_abi_json.rs
@@ -118,7 +118,7 @@ pub fn build(command: JsonAbiCommand) -> Result<Value, String> {
 
     if let Some(outfile) = json_outfile {
         let file = File::create(outfile).map_err(|e| e.to_string())?;
-        serde_json::to_writer(&file, &output_json.clone()).map_err(|e| e.to_string())?;
+        serde_json::to_writer(&file, &output_json).map_err(|e| e.to_string())?;
     } else {
         println!("{}", output_json);
     }
@@ -224,7 +224,7 @@ fn compile_dependency_lib<'source, 'manifest>(
         main_file,
         &manifest_of_dep.project.name,
         &dep_namespace,
-        build_config.clone(),
+        build_config,
         dependency_graph,
         silent_mode,
     )?;
@@ -235,7 +235,7 @@ fn compile_dependency_lib<'source, 'manifest>(
     Ok(json_abi)
 }
 
-fn compile_library<'source, 'manifest>(
+fn compile_library<'source>(
     source: &'source str,
     proj_name: &str,
     namespace: &Namespace<'source>,
@@ -275,15 +275,15 @@ fn compile_library<'source, 'manifest>(
     }
 }
 
-fn compile<'source, 'manifest>(
+fn compile<'source>(
     source: &'source str,
     proj_name: &str,
     namespace: &Namespace<'source>,
     build_config: BuildConfig,
     dependency_graph: &mut HashMap<String, HashSet<String>>,
     silent_mode: bool,
 ) -> Result<Vec<Function>, String> {
-    let res = sway_core::compile_to_ast(&source, namespace, &build_config, dependency_graph);
+    let res = sway_core::compile_to_ast(source, namespace, &build_config, dependency_graph);
     match res {
         CompileAstResult::Success {
             parse_tree,

diff --git a/forc/src/ops/forc_build.rs b/forc/src/ops/forc_build.rs
@@ -105,7 +105,7 @@ pub fn build(command: BuildCommand) -> Result<Vec<u8>, String> {
 
 /// Takes a dependency and returns a namespace of exported things from that dependency
 /// trait implementations are included as well
-fn compile_dependency_lib<'n, 'source, 'manifest>(
+fn compile_dependency_lib<'source, 'manifest>(
     project_file_path: &Path,
     dependency_name: &'manifest str,
     dependency_lib: &mut Dependency,
@@ -263,7 +263,7 @@ fn compile_library<'source>(
     }
 }
 
-fn compile<'n, 'source>(
+fn compile<'source>(
     source: &'source str,
     proj_name: &str,
     namespace: &Namespace<'source>,
@@ -275,11 +275,11 @@ fn compile<'n, 'source>(
     match res {
         BytecodeCompilationResult::Success { bytes, warnings } => {
             print_on_success(silent_mode, proj_name, warnings, TreeType::Script {});
-            return Ok(bytes);
+            Ok(bytes)
         }
         BytecodeCompilationResult::Library { warnings } => {
             print_on_success_library(silent_mode, proj_name, warnings);
-            return Ok(vec![]);
+            Ok(vec![])
         }
         BytecodeCompilationResult::Failure { errors, warnings } => {
             print_on_failure(silent_mode, warnings, errors);

diff --git a/forc/src/ops/forc_dep_check.rs b/forc/src/ops/forc_dep_check.rs
@@ -121,7 +121,7 @@ async fn check_untagged_dependency(
     dependency_name: &str,
     dep: &dependency::DependencyDetails,
 ) -> Result<()> {
-    let current = dependency::get_current_dependency_version(&target_directory)?;
+    let current = dependency::get_current_dependency_version(target_directory)?;
 
     let latest_hash = dependency::get_latest_commit_sha(git_repo, &dep.branch).await?;
 

diff --git a/forc/src/utils/helpers.rs b/forc/src/utils/helpers.rs
@@ -16,18 +16,15 @@ pub fn is_sway_file(file: &Path) -> bool {
     Some(OsStr::new(constants::SWAY_EXTENSION)) == res
 }
 
-pub fn find_main_path(manifest_dir: &PathBuf, manifest: &Manifest) -> PathBuf {
-    let mut code_dir = manifest_dir.clone();
+pub fn find_main_path(manifest_dir: &Path, manifest: &Manifest) -> PathBuf {
+    let mut code_dir = manifest_dir.to_path_buf();
     code_dir.push(constants::SRC_DIR);
     code_dir.push(&manifest.project.entry);
     code_dir
 }
 
-pub fn find_file_name<'sc>(
-    manifest_dir: &'sc PathBuf,
-    main_path: &'sc PathBuf,
-) -> Result<&'sc Path, String> {
-    let mut file_path = manifest_dir.clone();
+pub fn find_file_name<'sc>(manifest_dir: &Path, main_path: &'sc Path) -> Result<&'sc Path, String> {
+    let mut file_path = manifest_dir.to_path_buf();
     file_path.pop();
     let file_name = match main_path.strip_prefix(file_path.clone()) {
         Ok(o) => o,
@@ -90,7 +87,7 @@ pub fn print_on_success<'sc>(
     };
 
     if !silent_mode {
-        warnings.iter().for_each(|warning| format_warning(warning));
+        warnings.iter().for_each(format_warning);
     }
 
     if warnings.is_empty() {
@@ -110,13 +107,9 @@ pub fn print_on_success<'sc>(
     }
 }
 
-pub fn print_on_success_library<'sc>(
-    silent_mode: bool,
-    proj_name: &str,
-    warnings: Vec<CompileWarning>,
-) {
+pub fn print_on_success_library(silent_mode: bool, proj_name: &str, warnings: Vec<CompileWarning>) {
     if !silent_mode {
-        warnings.iter().for_each(|warning| format_warning(warning));
+        warnings.iter().for_each(format_warning);
     }
 
     if warnings.is_empty() {
@@ -143,7 +136,7 @@ pub fn print_on_failure(
     let e_len = errors.len();
 
     if !silent_mode {
-        warnings.iter().for_each(|warning| format_warning(warning));
+        warnings.iter().for_each(format_warning);
         errors.into_iter().for_each(|error| format_err(&error));
     }
 

diff --git a/sway-core/src/lib.rs b/sway-core/src/lib.rs
@@ -15,7 +15,7 @@ mod parser;
 pub mod semantic_analysis;
 mod span;
 mod style;
-pub(crate) mod type_engine;
+pub mod type_engine;
 
 use crate::asm_generation::checks::check_invalid_opcodes;
 pub use crate::parse_tree::*;

diff --git a/sway-core/src/semantic_analysis.rs b/sway-core/src/semantic_analysis.rs
@@ -9,6 +9,6 @@ pub use ast_node::{TypedConstantDeclaration, TypedDeclaration, TypedFunctionDecl
 pub use namespace::Namespace;
 pub use syntax_tree::TreeType;
 pub use syntax_tree::TypedParseTree;
-pub(crate) use type_check_arguments::*;
+pub use type_check_arguments::*;
 
 const ERROR_RECOVERY_DECLARATION: TypedDeclaration = TypedDeclaration::ErrorRecovery;
diff --git a/sway-core/src/semantic_analysis/type_check_arguments.rs b/sway-core/src/semantic_analysis/type_check_arguments.rs
@@ -6,17 +6,17 @@ use crate::type_engine::*;
 
 use std::collections::{HashMap, HashSet};
 pub struct TypeCheckArguments<'a, 'sc, T> {
-    pub(crate) checkee: T,
-    pub(crate) namespace: &'a mut Namespace<'sc>,
-    pub(crate) crate_namespace: Option<&'a Namespace<'sc>>,
-    pub(crate) return_type_annotation: TypeId,
-    pub(crate) help_text: &'static str,
-    pub(crate) self_type: TypeId,
-    pub(crate) build_config: &'a BuildConfig,
-    pub(crate) dead_code_graph: &'a mut ControlFlowGraph<'sc>,
-    pub(crate) mode: Mode,
-    pub(crate) dependency_graph: &'a mut HashMap<String, HashSet<String>>,
-    pub(crate) opts: TCOpts,
+    pub checkee: T,
+    pub namespace: &'a mut Namespace<'sc>,
+    pub crate_namespace: Option<&'a Namespace<'sc>>,
+    pub return_type_annotation: TypeId,
+    pub help_text: &'static str,
+    pub self_type: TypeId,
+    pub build_config: &'a BuildConfig,
+    pub dead_code_graph: &'a mut ControlFlowGraph<'sc>,
+    pub mode: Mode,
+    pub dependency_graph: &'a mut HashMap<String, HashSet<String>>,
+    pub opts: TCOpts,
 }
 
 #[derive(Default, Clone, Copy)]

diff --git a/sway-core/src/type_engine.rs b/sway-core/src/type_engine.rs
@@ -5,7 +5,7 @@ use std::iter::FromIterator;
 mod engine;
 mod integer_bits;
 mod type_info;
-pub(crate) use engine::*;
+pub use engine::*;
 pub use integer_bits::*;
 use sway_types::Property;
 pub use type_info::*;

diff --git a/sway-core/src/type_engine/engine.rs b/sway-core/src/type_engine/engine.rs
@@ -217,7 +217,7 @@ impl Engine {
     }
 }
 
-pub(crate) fn insert_type(ty: TypeInfo) -> TypeId {
+pub fn insert_type(ty: TypeInfo) -> TypeId {
     TYPE_ENGINE.insert_type(ty)
 }
 

diff --git a/sway-core/utils/selector_debug.rs b/sway-core/utils/selector_debug.rs
@@ -4,10 +4,12 @@ use pest::Parser;
 use structopt::StructOpt;
 
 use sway_core::{
-    error::CompileResult,
-    parse_tree::function_declaration::FunctionDeclaration,
-    semantic_analysis::ast_node::{declaration::TypedFunctionDeclaration, impl_trait::Mode},
-    type_engine::TypeInfo,
+    parse_tree::declaration::FunctionDeclaration,
+    semantic_analysis::{
+        ast_node::{declaration::TypedFunctionDeclaration, impl_trait::Mode},
+        TypeCheckArguments,
+    },
+    type_engine::*,
     BuildConfig, HllParser, Rule,
 };
 
@@ -20,47 +22,44 @@ struct Opt {
 fn main() {
     let opt = Opt::from_args();
     let fn_decl = opt.fn_decl;
+    let mut warnings = vec![];
+    let mut errors = vec![];
 
     let parsed_fn_decl = HllParser::parse(Rule::fn_decl, &fn_decl);
     let mut parsed_fn_decl = match parsed_fn_decl {
         Ok(o) => o,
         Err(e) => panic!("Failed to parse: {:?}", e),
     };
-    let parsed_fn_decl = FunctionDeclaration::parse_from_pair(parsed_fn_decl.next().unwrap());
-    let parsed_fn_decl = match parsed_fn_decl {
-        CompileResult::Ok { value, .. } => value,
-        CompileResult::Err { errors, .. } => panic!("Failed to parse: {:?}", errors),
-    };
+    let parsed_fn_decl =
+        FunctionDeclaration::parse_from_pair(parsed_fn_decl.next().unwrap(), Default::default())
+            .unwrap(&mut warnings, &mut errors);
 
-    let res = match TypedFunctionDeclaration::type_check(
-        parsed_fn_decl,
-        &Default::default(),
-        None,
-        "",
-        TypeInfo::Unit,
-        &BuildConfig::root_from_manifest_path(Default::default()),
-        &mut Default::default(),
-        Mode::ImplAbiFn,
-    ) {
-        CompileResult::Ok { value, .. } => value,
-        CompileResult::Err { errors, .. } => panic!("Failed to type check: {:?}", errors),
-    };
+    let res = TypedFunctionDeclaration::type_check(TypeCheckArguments {
+        checkee: parsed_fn_decl,
+        namespace: &mut Default::default(),
+        crate_namespace: None,
+        help_text: "",
+        return_type_annotation: insert_type(TypeInfo::Unknown),
+        self_type: insert_type(TypeInfo::Unknown),
+        build_config: &mut BuildConfig::root_from_file_name_and_manifest_path(
+            Default::default(),
+            Default::default(),
+        ),
+        dead_code_graph: &mut Default::default(),
+        mode: Mode::ImplAbiFn,
+        dependency_graph: &mut Default::default(),
+        opts: Default::default(),
+    })
+    .unwrap(&mut warnings, &mut errors);
 
-    let selector_string = match res.to_selector_name() {
-        CompileResult::Ok { value, .. } => value,
-        CompileResult::Err { errors, .. } => {
-            panic!("Failed to construct selector name: {:?}", errors)
-        }
-    };
-    let selector_hash_untruncated = match res.to_fn_selector_value_untruncated() {
-        CompileResult::Ok { value, .. } => value,
-        CompileResult::Err { errors, .. } => panic!("Failed to construct hash: {:?}", errors),
-    };
+    let selector_string = res.to_selector_name().unwrap(&mut warnings, &mut errors);
+    let selector_hash_untruncated = res
+        .to_fn_selector_value_untruncated()
+        .unwrap(&mut warnings, &mut errors);
     let selector_hash_untruncated = hex::encode(selector_hash_untruncated);
-    let selector_hash = match res.to_fn_selector_value() {
-        CompileResult::Ok { value, .. } => value,
-        CompileResult::Err { errors, .. } => panic!("Failed to construct hash: {:?}", errors),
-    };
+    let selector_hash = res
+        .to_fn_selector_value()
+        .unwrap(&mut warnings, &mut errors);
     let selector_hash = hex::encode(selector_hash);
     println!("selector string:         {}", selector_string);
     println!("untruncated hash:        0x{}", selector_hash_untruncated);

diff --git a/test/src/e2e_vm_tests/harness.rs b/test/src/e2e_vm_tests/harness.rs
@@ -81,7 +81,7 @@ pub(crate) fn runs_in_vm(file_name: &str) -> ProgramState {
     let block_height = (u32::MAX >> 1) as u64;
     tx_to_test.validate(block_height).unwrap();
     let mut i = Interpreter::with_storage(storage);
-    i.transact(tx_to_test).unwrap().state().clone()
+    *i.transact(tx_to_test).unwrap().state()
 }
 
 /// Panics if code _does_ compile, used for test cases where the source
-Original file line number
+Diff line change
@@ Expand Up / @@ -217,7 +217,7 @@ impl Engine { @@
         }
     }
-    pub(crate) fn insert_type(ty: TypeInfo) -> TypeId {
+    pub fn insert_type(ty: TypeInfo) -> TypeId {
         TYPE_ENGINE.insert_type(ty)
     }
@@ Expand Down @@