From 82c5d6b94aaaf0f605c8843bb55fd035c7e40509 Mon Sep 17 00:00:00 2001 From: Scott Carda <55811729+ScottCarda-MS@users.noreply.github.com> Date: Thu, 4 Apr 2024 10:05:32 -0700 Subject: [PATCH] Check Samples and Libraries for Correct Format in Pipeline (#1329) Expands on using the formatter from the command-line. - Allows folders to be given. - Adds the `--recursive` flag for specifying if you want the folders to be processed recursively. Without this, if given a folder it will only search the direct children of the folder. - Adds the `--write` flag for specifying that the formatter should overwrite unformatted Q# files with the format corrections. Without this flag, the formatter only checks if the Q# files are correctly formatted. To use the command-line formatter run `cargo run --binqsc_formatter -- `. Adds a check to the CI pipeline to run the formatter against the libraries and samples, and stops a build if it finds files that are not formatted correctly. --- .github/workflows/ci.yml | 17 +- Cargo.lock | 1 + build.py | 18 ++ compiler/qsc_formatter/Cargo.toml | 1 + compiler/qsc_formatter/src/main.rs | 273 +++++++++++++++++++++++++++-- 5 files changed, 297 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2a145cceaf..b775c2de17 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,6 +55,21 @@ jobs: - name: Clippy Lints run: cargo clippy --all-targets --all-features -- -D warnings + format-qsc: + name: Format Q# Files + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + with: + submodules: "true" + - name: Setup rust toolchain + uses: ./.github/actions/toolchains/rust + with: + toolchain: ${{ env.RUST_TOOLCHAIN_VERSION }} + components: ${{ env.RUST_TOOLCHAIN_COMPONENTS }} + - name: Check Formatting for Libraries and Samples + run: cargo run --release --bin qsc_formatter -- ./library/ ./samples/ -r + web-check: name: Check web files runs-on: ubuntu-latest @@ -207,7 +222,7 @@ jobs: status-check: name: Status Check - needs: [format, clippy, web-check, build, integration-tests, runBenchmark, runMemoryProfile] + needs: [format, clippy, web-check, build, format-qsc, integration-tests, runBenchmark, runMemoryProfile] runs-on: ubuntu-latest if: failure() steps: diff --git a/Cargo.lock b/Cargo.lock index b0d6081860..65557b0bc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1042,6 +1042,7 @@ dependencies = [ name = "qsc_formatter" version = "0.0.0" dependencies = [ + "clap", "expect-test", "indoc", "qsc_data_structures", diff --git a/build.py b/build.py index cc5e65cd6c..8fa7d13b55 100755 --- a/build.py +++ b/build.py @@ -193,6 +193,24 @@ def use_python_env(folder): text=True, cwd=root_dir, ) + + if build_cli or build_samples: + print("Running Q# format check") + subprocess.run( + [ + "cargo", + "run", + "--bin", + "qsc_formatter", + "--", + "./library/", + "./samples/", + "-r", + ], + check=True, + text=True, + cwd=root_dir, + ) step_end() if build_cli: diff --git a/compiler/qsc_formatter/Cargo.toml b/compiler/qsc_formatter/Cargo.toml index 9bc0f7544f..dcb40ad17a 100644 --- a/compiler/qsc_formatter/Cargo.toml +++ b/compiler/qsc_formatter/Cargo.toml @@ -9,6 +9,7 @@ edition.workspace = true license.workspace = true [dependencies] +clap = { workspace = true, features = ["derive", "cargo"] } qsc_data_structures = { path = "../qsc_data_structures" } qsc_frontend = { path = "../qsc_frontend" } diff --git a/compiler/qsc_formatter/src/main.rs b/compiler/qsc_formatter/src/main.rs index d6ab177724..ca65e0ace8 100644 --- a/compiler/qsc_formatter/src/main.rs +++ b/compiler/qsc_formatter/src/main.rs @@ -1,17 +1,266 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -use std::env; +use std::{ + env, + fmt::Display, + path::{Path, PathBuf}, +}; mod formatter; -use formatter::format_str; - -fn main() { - let args: Vec = env::args().collect(); - let path = args.get(1).expect("expected path to file to format"); - // read file from `path` into buffer - let file_as_string = std::fs::read_to_string(path).expect("file not found"); - // format the buffer - let formatted = format_str(&file_as_string); - // write the formatted buffer back to `path` - std::fs::write(path, formatted).expect("could not write to file"); +use clap::{crate_version, Parser}; +use formatter::{calculate_format_edits, format_str}; + +#[derive(Debug, Parser)] +#[command(name = "fmt", version = crate_version!())] +#[command(author, about, next_line_help = true)] +struct Cli { + /// Paths to the files or folders containing files to be formatted. + #[arg(required = true, num_args = 1..)] + paths: Vec, + + /// Search for Q# files recursively when a folder is given as PATH. + #[arg(short, long, default_value("false"))] + recursive: bool, + + /// Overwrite the file contents with the formatted contents. + #[arg(short, long, default_value("false"))] + write: bool, +} + +struct FileWalker { + roots: Vec, + is_write: bool, + is_recursive: bool, + file_count: i32, + changed_files: Vec, + skipped_files: Vec, +} + +impl FileWalker { + pub fn new() -> Self { + let cli = Cli::parse(); + Self { + roots: cli.paths, + is_write: cli.write, + is_recursive: cli.recursive, + file_count: 0, + changed_files: vec![], + skipped_files: vec![], + } + } + + fn format_from_roots(&mut self) { + let temp = self.roots.clone(); + for root in temp { + self.format_file_or_dir(&root); + } + } + + fn format_file_or_dir(&mut self, path: &Path) { + use OutputFormatting::*; + + if path.is_dir() { + let items = match path.read_dir() { + Ok(items) => items.flatten(), + Err(e) => { + println!("\t{Skip}Could not read from directory: {e}{Reset}"); + self.skipped_files.push(path.display().to_string()); + return; + } + }; + for item in items { + let subpath = &item.path(); + if subpath.is_dir() && self.is_recursive { + self.format_file_or_dir(subpath); + } else if is_path_qs(subpath) { + self.format_file(subpath); + } + } + } else if is_path_qs(path) { + self.format_file(path); + } + } + + fn format_file(&mut self, path: &Path) { + use OutputFormatting::*; + + if self.is_write { + println!("{Verb}Formatting{Reset} {}", path.display()); + } else { + println!("{Verb}Checking{Reset} {}", path.display()); + } + + let file_as_string = match std::fs::read_to_string(path) { + Ok(s) => s, + Err(e) => { + println!("\t{Skip}Could not read from file: {e}{Reset}"); + self.skipped_files.push(path.display().to_string()); + return; // don't count file in file_count + } + }; + if self.is_write { + let formatted = format_str(&file_as_string); + if file_as_string != formatted { + match std::fs::write(path, formatted) { + Ok(_) => { + self.changed_files.push(path.display().to_string()); + } + Err(e) => { + println!("\t{Skip}Could not write to file: {e}{Reset}"); + self.skipped_files.push(path.display().to_string()); + return; // don't count file in file_count + } + } + } + } else if !calculate_format_edits(&file_as_string).is_empty() { + self.changed_files.push(path.display().to_string()); + } + self.file_count += 1; + } +} + +enum OutputFormatting { + Error, + Skip, + Passing, + Verb, + Reset, +} + +impl Display for OutputFormatting { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + let output = match self { + OutputFormatting::Error => "\x1B[1;91m", // Bold, Bright Red + OutputFormatting::Passing => "\x1B[1;92m", // Bold, Bright Green + OutputFormatting::Skip => "\x1B[1;93m", // Bold, Bright Yellow + OutputFormatting::Verb => "\x1B[1;94m", // Bold, Bright Blue + OutputFormatting::Reset => "\x1B[0m", + }; + write!(f, "{output}") + } +} + +fn main() -> Result<(), String> { + use OutputFormatting::*; + + let mut file_walker = FileWalker::new(); + + for root in &file_walker.roots { + if !root.exists() { + return Err(format!("Given path {} can not found.", root.display())); + } + if !root.is_dir() && !is_path_qs(root) { + return Err(format!( + "Given path {} is not a folder or Q# file.", + root.display() + )); + } + } + + file_walker.format_from_roots(); + + println!("Ran against {} files.", file_walker.file_count); + + let are_skipped_files = !file_walker.skipped_files.is_empty(); + let are_changed_files = !file_walker.changed_files.is_empty(); + + if file_walker.is_write { + if are_changed_files { + println!( + "{Passing}Updated {} files:", + file_walker.changed_files.len() + ); + for f in file_walker.changed_files.iter() { + println!("\t{Passing}{f}"); + } + print!("{Reset}"); + } else { + println!("{Passing}No files updated.{Reset}"); + } + } else if are_changed_files { + println!( + "{Error}{} files are in need of formatting:", + file_walker.changed_files.len() + ); + for f in file_walker.changed_files.iter() { + println!("\t{Error}{f}"); + } + println!("{Error}Run the formatter with the `--write` option to correct formatting for the above files.{Reset}"); + } else { + println!( + "{Passing}{} files are correctly formatted.{Reset}", + file_walker.file_count + ); + } + if are_skipped_files { + println!("{Skip}Skipped {} files:", file_walker.skipped_files.len()); + for f in file_walker.skipped_files.iter() { + println!("\t{Skip}{f}"); + } + print!("{Reset}"); + } + + match (file_walker.is_write, are_changed_files, are_skipped_files) { + (true, _, false) // writing with no skips + | (false, false, false) => Ok(()), // checking with all formatted and no skips + (true, true, true) => { + println!( + "{Skip}Updated {} files. {} files skipped.{Reset}", + file_walker.changed_files.len(), + file_walker.skipped_files.len() + ); + Err(format!( + "Could not read/write from {} files", + file_walker.skipped_files.len() + )) + } + (true, false, true) => { + println!( + "{Skip}No files updated. {} files skipped.{Reset}", + file_walker.skipped_files.len() + ); + Err(format!( + "Could not read/write from {} files", + file_walker.skipped_files.len() + )) + } + (false, true, true) => { + println!( + "{Error}{} files are not formatted. {} files skipped.{Reset}", + file_walker.changed_files.len(), + file_walker.skipped_files.len() + ); + Err(format!( + "{} files are not formatted and could not read/write from {} files", + file_walker.changed_files.len(), + file_walker.skipped_files.len() + )) + } + (false, true, false) => Err(format!( + "{} files are not formatted", + file_walker.changed_files.len() + )), + (false, false, true) => { + println!( + "{Skip}{} files are correctly formatted. {} files skipped.{Reset}", + file_walker.file_count, + file_walker.skipped_files.len() + ); + Err(format!( + "Could not read/write from {} files", + file_walker.skipped_files.len() + )) + } + } +} + +fn is_path_qs(path: &Path) -> bool { + if path.is_file() { + if let Some(ex) = path.extension() { + if ex == "qs" { + return true; + } + } + } + false }