From dcb5dc954ea3a3bfb08c83956d2d242d282d09de Mon Sep 17 00:00:00 2001 From: John Lapeyre Date: Wed, 17 Jan 2024 15:52:44 -0500 Subject: [PATCH] Use option --all-targets with clippy check examples and tests This commit adds --all-targets to the local CI script as well as the remote CI script. It also fixes several violations found by the change. --- .github/workflows/main.yml | 4 +- crates/oq3_lexer/src/tests.rs | 6 ++- crates/oq3_semantics/examples/semdemo.rs | 17 +++---- crates/oq3_semantics/src/types.rs | 4 +- crates/oq3_semantics/tests/ast_tests.rs | 1 - .../oq3_semantics/tests/from_string_tests.rs | 15 +++---- crates/oq3_semantics/tests/types_test.rs | 24 +++++----- crates/oq3_syntax/examples/demotest.rs | 5 +-- crates/oq3_syntax/examples/itemparse.rs | 45 +++++++++---------- crates/oq3_syntax/examples/promote.rs | 2 +- crates/oq3_syntax/src/tests.rs | 5 +-- crates/oq3_syntax/src/tests/sourcegen_ast.rs | 14 +++--- local_CI.sh | 5 ++- 13 files changed, 69 insertions(+), 78 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index acdebb0..b90b18c 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -20,8 +20,8 @@ jobs: - name: Rust Format run: cargo fmt --all -- --check - name: Build - run: cargo build --verbose + run: cargo build --release --verbose - name: Clippy - run: cargo clippy -- -D warnings + run: cargo clippy --all-targets -- -D warnings - name: Run tests run: cargo test --verbose -- --skip sourcegen_ast --skip sourcegen_ast_nodes diff --git a/crates/oq3_lexer/src/tests.rs b/crates/oq3_lexer/src/tests.rs index 2f9b094..946e262 100644 --- a/crates/oq3_lexer/src/tests.rs +++ b/crates/oq3_lexer/src/tests.rs @@ -5,6 +5,8 @@ use super::*; use expect_test::{expect, Expect}; +// FIXME: Probably do what clippy asks for here +#[allow(clippy::format_collect)] fn check_lexing(src: &str, expect: Expect) { let actual: String = tokenize(src) .map(|token| format!("{:?}\n", token)) @@ -107,7 +109,7 @@ fn nested_block_comments() { #[test] fn literal_suffixes() { check_lexing( - r####" + r#" "a" 1234 0b101 @@ -120,7 +122,7 @@ fn literal_suffixes() { 6dt 5s 6dta -"####, +"#, expect![[r#" Token { kind: Whitespace, len: 1 } Token { kind: Literal { kind: Str { terminated: true }, suffix_start: 3 }, len: 3 } diff --git a/crates/oq3_semantics/examples/semdemo.rs b/crates/oq3_semantics/examples/semdemo.rs index 42d4b7f..1626b5a 100644 --- a/crates/oq3_semantics/examples/semdemo.rs +++ b/crates/oq3_semantics/examples/semdemo.rs @@ -7,9 +7,7 @@ use std::path::PathBuf; use oq3_lexer::{tokenize, Token}; use oq3_parser::SyntaxKind; -use oq3_semantics; use oq3_semantics::syntax_to_semantics; -use oq3_syntax; use oq3_syntax::{parse_text, GreenNode, SourceFile}; use rowan::NodeOrToken; // TODO: this can be accessed from a higher level @@ -84,7 +82,7 @@ fn main() { // matches just as you would the top level cmd match &cli.command { Some(Commands::SemanticString { file_name }) => { - let source = read_example_source(&file_name); + let source = read_example_source(file_name); let file_name = Some("giraffe"); let result = syntax_to_semantics::parse_source_string(source, file_name); if result.any_errors() { @@ -94,7 +92,7 @@ fn main() { } Some(Commands::Semantic { file_name }) => { - let result = syntax_to_semantics::parse_source_file(&file_name); + let result = syntax_to_semantics::parse_source_file(file_name); let have_errors = result.any_errors(); if have_errors { println!("Found errors: {}", have_errors); @@ -102,14 +100,14 @@ fn main() { } result.program().print_asg_debug(); dbg!(oq3_semantics::validate::count_symbol_errors( - &result.program(), - &result.symbol_table() + result.program(), + result.symbol_table() )); // dbg!(semantics::validate::count_symbol_errors(&result.program().stmts, &result.symbol_table())); } Some(Commands::SemanticPretty { file_name }) => { - let result = syntax_to_semantics::parse_source_file(&file_name); + let result = syntax_to_semantics::parse_source_file(file_name); println!("Found errors: {}", result.any_errors()); result.print_errors(); result.program().print_asg_debug_pretty(); @@ -163,9 +161,8 @@ fn main() { } fn read_example_source(file_path: &PathBuf) -> String { - let contents = fs::read_to_string(file_path.clone()) - .expect(format!("Unable to read file {:?}", file_path).as_str()); - return contents; + fs::read_to_string(file_path.clone()) + .unwrap_or_else(|_| panic!("Unable to read file {:?}", file_path)) } fn print_tree(file: SourceFile) { diff --git a/crates/oq3_semantics/src/types.rs b/crates/oq3_semantics/src/types.rs index bae069b..d996fa8 100644 --- a/crates/oq3_semantics/src/types.rs +++ b/crates/oq3_semantics/src/types.rs @@ -177,9 +177,9 @@ impl Type { #[test] fn test_type_enum1() { let t = Type::Bit(IsConst::False); - assert_eq!(t.is_const(), false); + assert!(!t.is_const()); assert!(t.width().is_none()); - assert_eq!(t.is_quantum(), false); + assert!(!t.is_quantum()); assert!(t.is_scalar()); } diff --git a/crates/oq3_semantics/tests/ast_tests.rs b/crates/oq3_semantics/tests/ast_tests.rs index 99621c1..b0f861c 100644 --- a/crates/oq3_semantics/tests/ast_tests.rs +++ b/crates/oq3_semantics/tests/ast_tests.rs @@ -1,7 +1,6 @@ // Copyright contributors to the openqasm-parser project // SPDX-License-Identifier: Apache-2.0 -use oq3_semantics; use oq3_semantics::asg; use oq3_semantics::symbols; use oq3_semantics::types; diff --git a/crates/oq3_semantics/tests/from_string_tests.rs b/crates/oq3_semantics/tests/from_string_tests.rs index a216f48..9bc3a03 100644 --- a/crates/oq3_semantics/tests/from_string_tests.rs +++ b/crates/oq3_semantics/tests/from_string_tests.rs @@ -13,9 +13,9 @@ fn parse_string(code: &str) -> (asg::Program, SemanticErrorList, SymbolTable) { #[test] fn test_from_string_one_qubit_decl() { - let code = r##" + let code = r#" qubit q; -"##; +"#; let (program, errors, symbol_table) = parse_string(code); assert!(errors.is_empty()); assert_eq!(program.len(), 1); @@ -51,7 +51,7 @@ if (true) { asg::Expr::Literal(asg::Literal::Bool(literal)) => literal, _ => unreachable!(), }; - assert_eq!(*bool_literal.value(), true); + assert!(*bool_literal.value()); let cond_ty = if_stmt.condition().get_type(); assert_eq!(cond_ty, &Type::Bool(IsConst::True)); @@ -79,13 +79,10 @@ while (false) { asg::Expr::Literal(asg::Literal::Bool(literal)) => literal, _ => unreachable!(), }; - assert_eq!(*bool_literal.value(), false); + assert!(!(*bool_literal.value())); let cond_ty = while_stmt.condition().get_type(); assert_eq!(cond_ty, &Type::Bool(IsConst::True)); let body_statements = while_stmt.loop_body().statements(); - match body_statements[1] { - _ => (), - }; let inner = body_statements .iter() .map(|x| match x { @@ -161,9 +158,9 @@ mygate(x, y) q; #[test] fn test_bit_string_literal() { - let code = r##" + let code = r#" bit[4] b = "1001"; -"##; +"#; let (program, errors, _symbol_table) = parse_string(code); assert_eq!(errors.len(), 0); assert_eq!(program.len(), 1); diff --git a/crates/oq3_semantics/tests/types_test.rs b/crates/oq3_semantics/tests/types_test.rs index 261c259..9b9cb5c 100644 --- a/crates/oq3_semantics/tests/types_test.rs +++ b/crates/oq3_semantics/tests/types_test.rs @@ -13,9 +13,9 @@ fn test_int_type_const() { let typ = Type::Int(Some(32), IsConst::True); assert_eq!(typ.width(), Some(32)); - assert_eq!(typ.is_scalar(), true); - assert_eq!(typ.is_const(), true); - assert_eq!(typ.is_quantum(), false); + assert!(typ.is_scalar()); + assert!(typ.is_const()); + assert!(!typ.is_quantum()); } #[test] @@ -24,9 +24,9 @@ fn test_int_type_not_const() { let typ = Type::Int(Some(32), IsConst::False); assert_eq!(typ.width(), Some(32)); - assert_eq!(typ.is_scalar(), true); - assert_eq!(typ.is_const(), false); - assert_eq!(typ.is_quantum(), false); + assert!(typ.is_scalar()); + assert!(!typ.is_const()); + assert!(!typ.is_quantum()); } #[test] @@ -35,9 +35,9 @@ fn test_int_type_no_width() { let typ = Type::Int(None, IsConst::False); // No width assert!(typ.width().is_none()); - assert_eq!(typ.is_scalar(), true); - assert_eq!(typ.is_const(), false); - assert_eq!(typ.is_quantum(), false); + assert!(typ.is_scalar()); + assert!(!typ.is_const()); + assert!(!typ.is_quantum()); } #[test] @@ -46,9 +46,9 @@ fn test_qubit_type_single_qubit() { let typ = Type::Qubit; assert!(typ.width().is_none()); - assert_eq!(typ.is_scalar(), false); - assert_eq!(typ.is_const(), true); - assert_eq!(typ.is_quantum(), true); + assert!(!typ.is_scalar()); + assert!(typ.is_const()); + assert!(typ.is_quantum()); } // #[test] diff --git a/crates/oq3_syntax/examples/demotest.rs b/crates/oq3_syntax/examples/demotest.rs index 8d8d8ff..1e89e36 100644 --- a/crates/oq3_syntax/examples/demotest.rs +++ b/crates/oq3_syntax/examples/demotest.rs @@ -105,9 +105,8 @@ fn example_path(example: &str) -> PathBuf { fn read_example_source(file_name: &str) -> String { let file_path = example_path(file_name); - let contents = fs::read_to_string(file_path.clone()) - .expect(format!("Unable to read file {:?}", file_path).as_str()); - return contents; + fs::read_to_string(file_path.clone()) + .unwrap_or_else(|_| panic!("Unable to read file {:?}", file_path)) } fn print_tree(file: SourceFile) { diff --git a/crates/oq3_syntax/examples/itemparse.rs b/crates/oq3_syntax/examples/itemparse.rs index b223b0d..f2c8b24 100644 --- a/crates/oq3_syntax/examples/itemparse.rs +++ b/crates/oq3_syntax/examples/itemparse.rs @@ -7,7 +7,7 @@ use oq3_syntax::SourceFile; #[allow(dead_code)] fn parse_some_code() { - let code = r##" + let code = r#" int q; OPENQASM 3.1; @@ -32,7 +32,7 @@ cal { x + y; } - "##; + "#; parse_print_items(code); } @@ -70,7 +70,7 @@ fn print_item(item: ast::Item) { fn parse_print_items(code: &str) { use oq3_syntax::AstNode; - let parse = SourceFile::parse(&code); + let parse = SourceFile::parse(code); let file: SourceFile = parse.tree(); println!("Found {} items", file.items().collect::>().len()); for item in file.items() { @@ -79,7 +79,7 @@ fn parse_print_items(code: &str) { item.syntax().descendants().collect::>().len() ); print_item(item.clone()); - println!(""); + println!(); // println!("{:?}", item.syntax().descendants().collect::>()); for d in item.syntax().descendants().collect::>() { // for d in item.children().collect::>() { @@ -104,24 +104,21 @@ gate mygate q { "##; // let parse = SourceFile::parse(&code); // let file: SourceFile = parse.tree(); - let file: SourceFile = SourceFile::parse(&code).tree(); + let file: SourceFile = SourceFile::parse(code).tree(); let mut gateitem = None; for item in file.items() { - match item { - ast::Item::Gate(gate) => { - gateitem = Some(gate); - break; - } - _ => (), - } + if let ast::Item::Gate(gate) = item { + gateitem = Some(gate); + break; + }; } println!("{}", test_gate_def(gateitem.unwrap(), ("mygate", "q"))); } #[allow(dead_code)] fn test_gate_def(gate: ast::Gate, (name, qubit_list): (&str, &str)) -> bool { - return format!("{}", gate.name().unwrap()) == name - && format!("{}", gate.qubit_params().unwrap()) == qubit_list; + format!("{}", gate.name().unwrap()) == name + && format!("{}", gate.qubit_params().unwrap()) == qubit_list } #[allow(dead_code)] @@ -147,7 +144,7 @@ fn print_type_declaration_statement(type_decl: ast::ClassicalDeclarationStatemen // println!(" none"); // } print!(" initial value: "); - if !type_decl.expr().is_none() { + if type_decl.expr().is_some() { print!("{}", type_decl.expr().unwrap()); } else { print!(" none"); @@ -161,7 +158,7 @@ fn print_type_declaration_statement(type_decl: ast::ClassicalDeclarationStatemen fn print_gate(gate: ast::Gate) { println!("Gate\ngate name: '{}'", gate.name().unwrap()); - if !gate.angle_params().is_none() { + if gate.angle_params().is_some() { println!("parameters: '{}'", gate.angle_params().unwrap()); } println!("qubits: '{}'", gate.qubit_params().unwrap()); @@ -170,11 +167,11 @@ fn print_gate(gate: ast::Gate) { fn print_defcal(defcal: ast::DefCal) { println!("DefCal\ndefcal name: '{}'", defcal.name().unwrap()); - if !defcal.param_list().is_none() { + if defcal.param_list().is_some() { println!("parameters: '{}'", defcal.param_list().unwrap()); } println!("qubits: '{}'", defcal.qubit_list().unwrap()); - if !defcal.ret_type().is_none() { + if defcal.ret_type().is_some() { println!("return type: '{}'", defcal.ret_type().unwrap()); } print!("body: '{}'", defcal.body().unwrap()); @@ -182,10 +179,10 @@ fn print_defcal(defcal: ast::DefCal) { fn print_def(def: ast::Def) { println!("Def\ndef name: '{}'", def.name().unwrap()); - if !def.param_list().is_none() { + if def.param_list().is_some() { println!("parameters: '{}'", def.param_list().unwrap()); } - if !def.ret_type().is_none() { + if def.ret_type().is_some() { println!("return type: '{}'", def.ret_type().unwrap()); } print!("body: '{}'", def.body().unwrap()); @@ -196,7 +193,7 @@ fn print_defcalgrammar(defcg: ast::DefCalGrammar) { "DefCalgrammar\ndefcalgrammar token: '{}'", defcg.defcalgrammar_token().unwrap() ); - if !defcg.file().is_none() { + if defcg.file().is_some() { print!("file: '{}'", defcg.file().unwrap()); } else { print!("file: NONE"); @@ -205,7 +202,7 @@ fn print_defcalgrammar(defcg: ast::DefCalGrammar) { fn print_cal(cal: ast::Cal) { println!("Cal\ncal token: '{}'", cal.cal_token().unwrap()); - if !cal.body().is_none() { + if cal.body().is_some() { print!("body: '{}'", cal.body().unwrap()); } else { print!("body: NONE"); @@ -217,7 +214,7 @@ fn print_version_string(version_string: ast::VersionString) { "VersionString\n openqasm_token: '{}'", version_string.OPENQASM_token().unwrap() ); - if !version_string.version().is_none() { + if version_string.version().is_some() { print!("version: '{}'", version_string.version().unwrap()); } else { print!("version: NONE"); @@ -229,7 +226,7 @@ fn print_include(include: ast::Include) { "Include\ninclude token: '{}'", include.include_token().unwrap() ); - if !include.file().is_none() { + if include.file().is_some() { print!("file: '{}'", include.file().unwrap()); } else { print!("file: NONE"); diff --git a/crates/oq3_syntax/examples/promote.rs b/crates/oq3_syntax/examples/promote.rs index 7f26181..3b38a32 100644 --- a/crates/oq3_syntax/examples/promote.rs +++ b/crates/oq3_syntax/examples/promote.rs @@ -148,7 +148,7 @@ fn _one_call(op: BinOp, type1: Type, type2: Type) -> Option { otype = promote_type(type2, type1); } assert!(otype != Bottom); - return Some(otype); + Some(otype) } // Analyze a vector of triples (Binary op, type of first arg, type of second arg). diff --git a/crates/oq3_syntax/src/tests.rs b/crates/oq3_syntax/src/tests.rs index 4f6c017..86de5ab 100644 --- a/crates/oq3_syntax/src/tests.rs +++ b/crates/oq3_syntax/src/tests.rs @@ -243,9 +243,8 @@ defcal xmeasure(int a, int b) q, p -> bit { let mut defcal = None; for item in file.items() { - match item { - ast::Item::DefCal(f) => defcal = Some(f), - _ => (), + if let ast::Item::DefCal(f) = item { + defcal = Some(f) } } let defcal: ast::DefCal = defcal.unwrap(); diff --git a/crates/oq3_syntax/src/tests/sourcegen_ast.rs b/crates/oq3_syntax/src/tests/sourcegen_ast.rs index 5170e62..30b8ce5 100644 --- a/crates/oq3_syntax/src/tests/sourcegen_ast.rs +++ b/crates/oq3_syntax/src/tests/sourcegen_ast.rs @@ -14,7 +14,6 @@ use std::{ use itertools::Itertools; use proc_macro2::{Punct, Spacing}; use quote::{format_ident, quote}; -use std::path::PathBuf; use ungrammar::{Grammar, Rule}; use crate::sourcegen as localsourcegen; @@ -44,7 +43,7 @@ fn _generate_ast() -> AstSrc { .unwrap(); let ast = lower(&grammar); println!("AST: {:?}", ast); - return ast; + ast } /// Generate the code destined for nodes.rs, but write to temp file _new_nodes.rs. @@ -414,12 +413,11 @@ fn generate_syntax_kinds(grammar: KindsSrc<'_>) -> String { // let contextual_keywords_values = &grammar.contextual_keywords; // let contextual_keywords = contextual_keywords_values.iter().map(upper_snake); - let all_keywords_values = grammar - .keywords - .iter() - // .chain(grammar.contextual_keywords.iter()) - .copied() - .collect::>(); + let all_keywords_values = grammar.keywords.to_vec(); + // .iter() + // // .chain(grammar.contextual_keywords.iter()) + // .copied() + // .collect::>(); let all_keywords_idents = all_keywords_values.iter().map(|kw| format_ident!("{}", kw)); let all_keywords = all_keywords_values .iter() diff --git a/local_CI.sh b/local_CI.sh index 81c99c0..c559d58 100755 --- a/local_CI.sh +++ b/local_CI.sh @@ -3,4 +3,7 @@ # Run locally the same checks that are done remotely in CI # Works on linux, and maybe Mac OS. -cargo fmt --all -- --check && cargo build --verbose && cargo clippy -- -D warnings && cargo test --verbose -- --skip sourcegen_ast --skip sourcegen_ast_nodes +cargo fmt --all -- --check || exit 1 +cargo build --release --verbose || exit 1 +cargo clippy --all-targets -- -D warnings || exit 1 +cargo test --verbose -- --skip sourcegen_ast --skip sourcegen_ast_nodes || exit 1