diff --git a/.github/workflows/binary-builds.yml b/.github/workflows/binary-builds.yml index 85ed176..87280bb 100644 --- a/.github/workflows/binary-builds.yml +++ b/.github/workflows/binary-builds.yml @@ -109,7 +109,7 @@ jobs: run: >- ${{ matrix.cross && 'cross' || 'cargo '}} build - --manifest-path cpp-linter-lib/Cargo.toml + --manifest-path cpp-linter/Cargo.toml --bin cpp-linter --release --target ${{ matrix.target }} diff --git a/.github/workflows/build-docs.yml b/.github/workflows/build-docs.yml index 1e036f9..31f7262 100644 --- a/.github/workflows/build-docs.yml +++ b/.github/workflows/build-docs.yml @@ -34,11 +34,11 @@ jobs: - name: Install mdbook uses: taiki-e/install-action@v2 with: - tool: mdbook,cargo-binstall + tool: mdbook,cargo-binstall,just - name: Install mdbook plugins run: cargo binstall -y mdbook-alerts - name: Build book - run: mdbook build docs + run: just docs-build - name: Upload docs build as artifact uses: actions/upload-artifact@v4 with: @@ -63,7 +63,11 @@ jobs: with: path: ~/.cargo key: ${{ runner.os }}-docs-cargo-${{ hashFiles('Cargo.lock') }} - - run: cargo doc --no-deps --manifest-path cpp-linter-lib/Cargo.toml + - name: Install mdbook + uses: taiki-e/install-action@v2 + with: + tool: just + - run: just docs-rs - name: upload rustdoc build as artifact uses: actions/upload-artifact@v4 with: diff --git a/.github/workflows/python-packaging.yml b/.github/workflows/python-packaging.yml index cb380d0..29c2d47 100644 --- a/.github/workflows/python-packaging.yml +++ b/.github/workflows/python-packaging.yml @@ -53,7 +53,7 @@ jobs: uses: PyO3/maturin-action@v1 with: target: ${{ matrix.target }} - args: --manifest-path cpp-linter-py/Cargo.toml --release --out dist --find-interpreter ${{ steps.is-openssl-vendored.outputs.enabled }} + args: --manifest-path py-binding/Cargo.toml --release --out dist --find-interpreter ${{ steps.is-openssl-vendored.outputs.enabled }} manylinux: auto before-script-linux: | case "${{ matrix.target }}" in @@ -90,7 +90,7 @@ jobs: uses: PyO3/maturin-action@v1 with: target: ${{ matrix.target }} - args: --manifest-path cpp-linter-py/Cargo.toml --release --out dist --find-interpreter + args: --manifest-path py-binding/Cargo.toml --release --out dist --find-interpreter - name: Upload wheels uses: actions/upload-artifact@v4 with: @@ -112,7 +112,7 @@ jobs: uses: PyO3/maturin-action@v1 with: target: ${{ matrix.target }} - args: --manifest-path cpp-linter-py/Cargo.toml --release --out dist --find-interpreter --features openssl-vendored + args: --manifest-path py-binding/Cargo.toml --release --out dist --find-interpreter --features openssl-vendored - name: Upload wheels uses: actions/upload-artifact@v4 with: @@ -127,7 +127,7 @@ jobs: uses: PyO3/maturin-action@v1 with: command: sdist - args: --manifest-path cpp-linter-py/Cargo.toml --out dist + args: --manifest-path py-binding/Cargo.toml --out dist - name: Upload sdist uses: actions/upload-artifact@v4 with: diff --git a/.github/workflows/run-dev-tests.yml b/.github/workflows/run-dev-tests.yml index 688a381..dbb794f 100644 --- a/.github/workflows/run-dev-tests.yml +++ b/.github/workflows/run-dev-tests.yml @@ -214,7 +214,9 @@ jobs: - name: Generate Coverage lcov report if: runner.os == 'Linux' - run: just lcov + run: | + rm coverage.json + just lcov - uses: codecov/codecov-action@v4 if: runner.os == 'Linux' diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 21aeeac..c2a90f6 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -3,7 +3,7 @@ repos: rev: v4.6.0 hooks: - id: trailing-whitespace - exclude: cpp-linter-lib/tests/.*\.(?:patch|diff) + exclude: cpp-linter/tests/.*\.(?:patch|diff) - id: end-of-file-fixer - id: check-docstring-first - id: check-added-large-files diff --git a/Cargo.lock b/Cargo.lock index d74c513..6439dec 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -279,7 +279,7 @@ name = "cli-gen" version = "0.1.0" dependencies = [ "clap", - "cpp-linter-lib", + "cpp-linter", "mdbook", "semver", "serde_json", @@ -318,7 +318,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "06ea2b9bc92be3c2baa9334a323ebca2d6f074ff852cd1d7b11064035cd3868f" [[package]] -name = "cpp-linter-lib" +name = "cpp-linter" version = "2.0.0-rc1" dependencies = [ "chrono", @@ -348,7 +348,7 @@ dependencies = [ name = "cpp-linter-py" version = "2.0.0-rc1" dependencies = [ - "cpp-linter-lib", + "cpp-linter", "pyo3", "tokio", ] diff --git a/Cargo.toml b/Cargo.toml index 3e270e8..8cbadb7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [workspace] -members = ["cpp-linter-lib", "cpp-linter-py", "docs"] +members = ["cpp-linter", "py-binding", "docs"] resolver = "2" [workspace.package] diff --git a/cpp-linter-lib/src/rest_api/github_api.rs b/cpp-linter-lib/src/rest_api/github_api.rs deleted file mode 100644 index bd99209..0000000 --- a/cpp-linter-lib/src/rest_api/github_api.rs +++ /dev/null @@ -1,952 +0,0 @@ -//! This module holds functionality specific to using Github's REST API. - -use std::collections::HashMap; -use std::env; -use std::fs::OpenOptions; -use std::io::{Read, Write}; -use std::sync::{Arc, Mutex}; - -// non-std crates -use reqwest::header::{HeaderMap, HeaderValue, AUTHORIZATION}; -use reqwest::Method; -use reqwest::{Client, Url}; -use serde::{Deserialize, Serialize}; -use serde_json; - -// project specific modules/crates -use crate::clang_tools::clang_format::{summarize_style, tally_format_advice}; -use crate::clang_tools::clang_tidy::tally_tidy_advice; -use crate::clang_tools::{ReviewComments, Suggestion}; -use crate::cli::{FeedbackInput, ThreadComments}; -use crate::common_fs::{FileFilter, FileObj}; -use crate::git::{get_diff, open_repo, parse_diff, parse_diff_from_buf}; - -use super::{RestApiClient, RestApiRateLimitHeaders, COMMENT_MARKER}; - -/// A structure to work with Github REST API. -pub struct GithubApiClient { - /// The HTTP request client to be used for all REST API calls. - client: Client, - - /// The CI run's event payload from the webhook that triggered the workflow. - pull_request: Option, - - /// The name of the event that was triggered when running cpp_linter. - pub event_name: String, - - /// The value of the `GITHUB_API_URL` environment variable. - api_url: Url, - - /// The value of the `GITHUB_REPOSITORY` environment variable. - repo: Option, - - /// The value of the `GITHUB_SHA` environment variable. - sha: Option, - - /// The value of the `ACTIONS_STEP_DEBUG` environment variable. - pub debug_enabled: bool, - - rate_limit_headers: RestApiRateLimitHeaders, -} - -impl Default for GithubApiClient { - fn default() -> Self { - Self::new() - } -} - -impl GithubApiClient { - pub fn new() -> Self { - let event_name = env::var("GITHUB_EVENT_NAME").unwrap_or(String::from("unknown")); - let pull_request = { - match event_name.as_str() { - "pull_request" => { - let event_payload_path = env::var("GITHUB_EVENT_PATH") - .expect("GITHUB_EVENT_NAME is set to 'pull_request', but GITHUB_EVENT_PATH is not set"); - let file_buf = &mut String::new(); - OpenOptions::new() - .read(true) - .open(event_payload_path) - .unwrap() - .read_to_string(file_buf) - .unwrap(); - let json = serde_json::from_str::>( - file_buf, - ) - .unwrap(); - json["number"].as_i64() - } - _ => None, - } - }; - let api_url = Url::parse( - env::var("GITHUB_API_URL") - .unwrap_or("https://api.github.com".to_string()) - .as_str(), - ) - .expect("Failed to parse URL from GITHUB_API_URL"); - - GithubApiClient { - client: Client::builder() - .default_headers(Self::make_headers()) - .build() - .expect("Failed to create a session client for REST API calls"), - pull_request, - event_name, - api_url, - repo: match env::var("GITHUB_REPOSITORY") { - Ok(val) => Some(val), - Err(_) => None, - }, - sha: match env::var("GITHUB_SHA") { - Ok(val) => Some(val), - Err(_) => None, - }, - debug_enabled: match env::var("ACTIONS_STEP_DEBUG") { - Ok(val) => val == "true", - Err(_) => false, - }, - rate_limit_headers: RestApiRateLimitHeaders { - reset: "x-ratelimit-reset".to_string(), - remaining: "x-ratelimit-remaining".to_string(), - retry: "retry-after".to_string(), - }, - } - } -} - -// implement the RestApiClient trait for the GithubApiClient -impl RestApiClient for GithubApiClient { - fn set_exit_code( - &self, - checks_failed: u64, - format_checks_failed: Option, - tidy_checks_failed: Option, - ) -> u64 { - if let Ok(gh_out) = env::var("GITHUB_OUTPUT") { - let mut gh_out_file = OpenOptions::new() - .append(true) - .open(gh_out) - .expect("GITHUB_OUTPUT file could not be opened"); - for (prompt, value) in [ - ("checks-failed", Some(checks_failed)), - ("format-checks-failed", format_checks_failed), - ("tidy-checks-failed", tidy_checks_failed), - ] { - if let Err(e) = writeln!(gh_out_file, "{prompt}={}", value.unwrap_or(0),) { - log::error!("Could not write to GITHUB_OUTPUT file: {}", e); - break; - } - } - gh_out_file - .flush() - .expect("Failed to flush buffer to GITHUB_OUTPUT file"); - } - log::info!( - "{} clang-format-checks-failed", - format_checks_failed.unwrap_or(0) - ); - log::info!( - "{} clang-tidy-checks-failed", - tidy_checks_failed.unwrap_or(0) - ); - log::info!("{checks_failed} checks-failed"); - checks_failed - } - - fn make_headers() -> HeaderMap { - let mut headers = HeaderMap::new(); - headers.insert( - "Accept", - HeaderValue::from_str("application/vnd.github.raw+json") - .expect("Failed to create a header value for the API return data type"), - ); - // headers.insert("User-Agent", USER_AGENT.parse().unwrap()); - if let Ok(token) = env::var("GITHUB_TOKEN") { - let mut val = HeaderValue::from_str(token.as_str()) - .expect("Failed to create a secure header value for the API token."); - val.set_sensitive(true); - headers.insert(AUTHORIZATION, val); - } - headers - } - - async fn get_list_of_changed_files(&self, file_filter: &FileFilter) -> Vec { - if env::var("CI").is_ok_and(|val| val.as_str() == "true") - && self.repo.is_some() - && self.sha.is_some() - { - // get diff from Github REST API - let is_pr = self.event_name == "pull_request"; - let pr = self.pull_request.unwrap_or(-1).to_string(); - let sha = self.sha.clone().unwrap(); - let url = self - .api_url - .join("repos/") - .unwrap() - .join(format!("{}/", self.repo.as_ref().unwrap()).as_str()) - .unwrap() - .join(if is_pr { "pulls/" } else { "commits/" }) - .unwrap() - .join(if is_pr { pr.as_str() } else { sha.as_str() }) - .unwrap(); - let mut diff_header = HeaderMap::new(); - diff_header.insert("Accept", "application/vnd.github.diff".parse().unwrap()); - let request = Self::make_api_request( - &self.client, - url.as_str(), - Method::GET, - None, - Some(diff_header), - ); - let response = Self::send_api_request( - self.client.clone(), - request, - false, - self.rate_limit_headers.to_owned(), - 0, - ) - .await; - match response { - Some(response) => { - if response.status.is_success() { - return parse_diff_from_buf(response.text.as_bytes(), file_filter); - } else { - let endpoint = if is_pr { - Url::parse(format!("{}/files", url.as_str()).as_str()) - .expect("failed to parse URL endpoint") - } else { - url - }; - self.get_changed_files_paginated(endpoint, file_filter) - .await - } - } - None => { - panic!("Failed to connect with GitHub server to get list of changed files.") - } - } - } else { - // get diff from libgit2 API - let repo = open_repo(".") - .expect("Please ensure the repository is checked out before running cpp-linter."); - let list = parse_diff(&get_diff(&repo), file_filter); - list - } - } - - async fn get_changed_files_paginated( - &self, - url: Url, - file_filter: &FileFilter, - ) -> Vec { - let mut url = Some(Url::parse_with_params(url.as_str(), &[("page", "1")]).unwrap()); - let mut files = vec![]; - while let Some(ref endpoint) = url { - let request = - Self::make_api_request(&self.client, endpoint.as_str(), Method::GET, None, None); - let response = Self::send_api_request( - self.client.clone(), - request, - true, - self.rate_limit_headers.clone(), - 0, - ) - .await; - if let Some(response) = response { - url = Self::try_next_page(&response.headers); - let files_list = if self.event_name != "pull_request" { - let json_value: PushEventFiles = serde_json::from_str(&response.text) - .expect("Failed to deserialize list of changed files from json response"); - json_value.files - } else { - serde_json::from_str::>(&response.text).expect( - "Failed to deserialize list of file changes from Pull Request event.", - ) - }; - for file in files_list { - if let Some(patch) = file.patch { - let diff = format!( - "diff --git a/{old} b/{new}\n--- a/{old}\n+++ b/{new}\n{patch}", - old = file.previous_filename.unwrap_or(file.filename.clone()), - new = file.filename, - ); - if let Some(file_obj) = - parse_diff_from_buf(diff.as_bytes(), file_filter).first() - { - files.push(file_obj.to_owned()); - } - } - } - } - } - files - } - - async fn post_feedback( - &self, - files: &[Arc>], - feedback_inputs: FeedbackInput, - ) -> u64 { - let tidy_checks_failed = tally_tidy_advice(files); - let format_checks_failed = tally_format_advice(files); - let mut comment = None; - - if feedback_inputs.file_annotations { - self.post_annotations(files, feedback_inputs.style.as_str()); - } - if feedback_inputs.step_summary { - comment = - Some(self.make_comment(files, format_checks_failed, tidy_checks_failed, None)); - self.post_step_summary(comment.as_ref().unwrap()); - } - self.set_exit_code( - format_checks_failed + tidy_checks_failed, - Some(format_checks_failed), - Some(tidy_checks_failed), - ); - - if feedback_inputs.thread_comments != ThreadComments::Off { - // post thread comment for PR or push event - if comment.as_ref().is_some_and(|c| c.len() > 65535) || comment.is_none() { - comment = Some(self.make_comment( - files, - format_checks_failed, - tidy_checks_failed, - Some(65535), - )); - } - if let Some(repo) = &self.repo { - let is_pr = self.event_name == "pull_request"; - let pr = self.pull_request.unwrap_or(-1).to_string() + "/"; - let sha = self.sha.clone().unwrap() + "/"; - let comments_url = self - .api_url - .join("repos/") - .unwrap() - .join(format!("{}/", repo).as_str()) - .unwrap() - .join(if is_pr { "issues/" } else { "commits/" }) - .unwrap() - .join(if is_pr { pr.as_str() } else { sha.as_str() }) - .unwrap() - .join("comments/") - .unwrap(); - - self.update_comment( - comments_url, - &comment.unwrap(), - feedback_inputs.no_lgtm, - format_checks_failed + tidy_checks_failed == 0, - feedback_inputs.thread_comments == ThreadComments::Update, - ) - .await; - } - } - if self.event_name == "pull_request" - && (feedback_inputs.tidy_review || feedback_inputs.format_review) - { - self.post_review(files, &feedback_inputs).await; - } - format_checks_failed + tidy_checks_failed - } -} - -impl GithubApiClient { - fn post_step_summary(&self, comment: &String) { - if let Ok(gh_out) = env::var("GITHUB_STEP_SUMMARY") { - let mut gh_out_file = OpenOptions::new() - .append(true) - .open(gh_out) - .expect("GITHUB_STEP_SUMMARY file could not be opened"); - if let Err(e) = writeln!(gh_out_file, "\n{}\n", comment) { - log::error!("Could not write to GITHUB_STEP_SUMMARY file: {}", e); - } - } - } - - fn post_annotations(&self, files: &[Arc>], style: &str) { - let style_guide = summarize_style(style); - - // iterate over clang-format advice and post annotations - for file in files { - let file = file.lock().unwrap(); - if let Some(format_advice) = &file.format_advice { - // assemble a list of line numbers - let mut lines: Vec = Vec::new(); - for replacement in &format_advice.replacements { - if let Some(line_int) = replacement.line { - if !lines.contains(&line_int) { - lines.push(line_int); - } - } - } - // post annotation if any applicable lines were formatted - if !lines.is_empty() { - println!( - "::notice file={name},title=Run clang-format on {name}::File {name} does not conform to {style_guide} style guidelines. (lines {line_set})", - name = &file.name.to_string_lossy().replace('\\', "/"), - line_set = lines.iter().map(|val| val.to_string()).collect::>().join(","), - ); - } - } // end format_advice iterations - - // iterate over clang-tidy advice and post annotations - // The tidy_advice vector is parallel to the files vector; meaning it serves as a file filterer. - // lines are already filter as specified to clang-tidy CLI. - if let Some(tidy_advice) = &file.tidy_advice { - for note in &tidy_advice.notes { - if note.filename == file.name.to_string_lossy().replace('\\', "/") { - println!( - "::{severity} file={file},line={line},title={file}:{line}:{cols} [{diag}]::{info}", - severity = if note.severity == *"note" { "notice".to_string() } else {note.severity.clone()}, - file = note.filename, - line = note.line, - cols = note.cols, - diag = note.diagnostic, - info = note.rationale, - ); - } - } - } - } - } - - /// update existing comment or remove old comment(s) and post a new comment - async fn update_comment( - &self, - url: Url, - comment: &String, - no_lgtm: bool, - is_lgtm: bool, - update_only: bool, - ) { - let comment_url = self - .remove_bot_comments(&url, !update_only || (is_lgtm && no_lgtm)) - .await; - #[allow(clippy::nonminimal_bool)] // an inaccurate assessment - if (is_lgtm && !no_lgtm) || !is_lgtm { - let payload = HashMap::from([("body", comment.to_owned())]); - // log::debug!("payload body:\n{:?}", payload); - let req_meth = if comment_url.is_some() { - Method::PATCH - } else { - Method::POST - }; - let request = Self::make_api_request( - &self.client, - if let Some(url_) = comment_url { - url_ - } else { - url - }, - req_meth, - Some( - serde_json::to_string(&payload) - .expect("Failed to serialize thread comment to json string"), - ), - None, - ); - Self::send_api_request( - self.client.clone(), - request, - false, - self.rate_limit_headers.to_owned(), - 0, - ) - .await; - } - } - - async fn remove_bot_comments(&self, url: &Url, delete: bool) -> Option { - let mut comment_url = None; - let mut comments_url = Some( - Url::parse_with_params(url.as_str(), &[("page", "1")]) - .expect("Failed to parse invalid URL string"), - ); - let repo = format!( - "repos/{}/comments/", - self.repo.as_ref().expect("Repo name unknown.") - ); - let base_comment_url = self.api_url.join(&repo).unwrap(); - while let Some(ref endpoint) = comments_url { - let request = - Self::make_api_request(&self.client, endpoint.as_str(), Method::GET, None, None); - let response = Self::send_api_request( - self.client.clone(), - request, - false, - self.rate_limit_headers.to_owned(), - 0, - ) - .await; - if response.is_none() || response.as_ref().is_some_and(|r| !r.status.is_success()) { - log::error!("Failed to get list of existing comments from {}", endpoint); - return comment_url; - } - comments_url = Self::try_next_page(&response.as_ref().unwrap().headers); - let payload: Vec = serde_json::from_str(&response.unwrap().text) - .expect("Failed to serialize response's text"); - for comment in payload { - if comment.body.starts_with(COMMENT_MARKER) { - log::debug!( - "comment id {} from user {} ({})", - comment.id, - comment.user.login, - comment.user.id, - ); - #[allow(clippy::nonminimal_bool)] // an inaccurate assessment - if delete || (!delete && comment_url.is_some()) { - // if not updating: remove all outdated comments - // if updating: remove all outdated comments except the last one - - // use last saved comment_url (if not None) or current comment url - let del_url = if let Some(last_url) = &comment_url { - last_url - } else { - let comment_id = comment.id.to_string(); - &base_comment_url - .join(&comment_id) - .expect("Failed to parse URL from JSON comment.url") - }; - let req = Self::make_api_request( - &self.client, - del_url.clone(), - Method::DELETE, - None, - None, - ); - Self::send_api_request( - self.client.clone(), - req, - false, - self.rate_limit_headers.to_owned(), - 0, - ) - .await; - } - if !delete { - let comment_id = comment.id.to_string(); - comment_url = Some( - base_comment_url - .join(&comment_id) - .expect("Failed to parse URL from JSON comment.url"), - ) - } - } - } - } - comment_url - } - - /// Post a PR review with code suggestions. - /// - /// Note: `--no-lgtm` is applied when nothing is suggested. - pub async fn post_review(&self, files: &[Arc>], feedback_input: &FeedbackInput) { - let url = self - .api_url - .join("repos/") - .unwrap() - .join(format!("{}/", self.repo.as_ref().expect("Repo name unknown")).as_str()) - .unwrap() - .join("pulls/") - .unwrap() - .join( - self.pull_request - .expect("pull request number unknown") - .to_string() - .as_str(), - ) - .unwrap(); - let request = Self::make_api_request(&self.client, url.as_str(), Method::GET, None, None); - let response = Self::send_api_request( - self.client.clone(), - request, - true, - self.rate_limit_headers.clone(), - 0, - ) - .await; - let pr_info: PullRequestInfo = - serde_json::from_str(&response.expect("Failed to get PR info").text) - .expect("Failed to deserialize PR info"); - - let url = Url::parse(format!("{}/", url.as_str()).as_str()) - .unwrap() - .join("reviews") - .expect("Failed to parse URL endpoint for PR reviews"); - let dismissal = self.dismiss_outdated_reviews(&url); - - if pr_info.draft || pr_info.state != "open" { - dismissal.await; - return; - } - - let summary_only = - env::var("CPP_LINTER_PR_REVIEW_SUMMARY_ONLY").unwrap_or("false".to_string()) == "true"; - - let mut review_comments = ReviewComments::default(); - for file in files { - let file = file.lock().unwrap(); - file.make_suggestions_from_patch(&mut review_comments, summary_only); - } - let has_no_changes = - review_comments.full_patch[0].is_empty() && review_comments.full_patch[1].is_empty(); - if has_no_changes && feedback_input.no_lgtm { - log::debug!("Not posting an approved review because `no-lgtm` is true"); - dismissal.await; - return; - } - let mut payload = FullReview { - event: if feedback_input.passive_reviews { - String::from("COMMENT") - } else if has_no_changes { - String::from("APPROVE") - } else { - String::from("REQUEST_CHANGES") - }, - body: String::new(), - comments: vec![], - }; - payload.body = review_comments.summarize(); - if !summary_only { - payload.comments = { - let mut comments = vec![]; - for comment in review_comments.comments { - comments.push(ReviewDiffComment::from(comment)); - } - comments - }; - } - dismissal.await; // free up the `url` variable - let request = Self::make_api_request( - &self.client, - url, - Method::POST, - Some( - serde_json::to_string(&payload) - .expect("Failed to serialize PR review to json string"), - ), - None, - ); - let response = Self::send_api_request( - self.client.clone(), - request, - false, - self.rate_limit_headers.clone(), - 0, - ) - .await; - if response.is_none() || response.is_some_and(|r| !r.status.is_success()) { - log::error!("Failed to post a new PR review"); - } - } - - /// Dismiss any outdated reviews generated by cpp-linter. - async fn dismiss_outdated_reviews(&self, url: &Url) { - let mut url_ = Some( - Url::parse_with_params(url.as_str(), [("page", "1")]) - .expect("Failed to parse endpoint for getting existing PR reviews"), - ); - while let Some(ref endpoint) = url_ { - let request = - Self::make_api_request(&self.client, endpoint.as_str(), Method::GET, None, None); - let response = Self::send_api_request( - self.client.clone(), - request, - false, - self.rate_limit_headers.clone(), - 0, - ) - .await; - if response.is_none() || response.as_ref().is_some_and(|r| !r.status.is_success()) { - log::error!("Failed to get a list of existing PR reviews"); - return; - } - let response = response.unwrap(); - url_ = Self::try_next_page(&response.headers); - let payload: Vec = serde_json::from_str(&response.text) - .expect("Unable to deserialize malformed JSON about review comments"); - for review in payload { - if let Some(body) = &review.body { - if body.starts_with(COMMENT_MARKER) - && !(["PENDING", "DISMISSED"].contains(&review.state.as_str())) - { - // dismiss outdated review - let req = Self::make_api_request( - &self.client, - url.join("reviews/") - .unwrap() - .join(review.id.to_string().as_str()) - .expect("Failed to parse URL for dismissing outdated review."), - Method::PUT, - Some( - serde_json::json!( - { - "message": "outdated suggestion", - "event": "DISMISS" - } - ) - .to_string(), - ), - None, - ); - let result = Self::send_api_request( - self.client.clone(), - req, - false, - self.rate_limit_headers.clone(), - 0, - ) - .await; - if result.is_none() || result.is_some_and(|r| !r.status.is_success()) { - log::error!("Failed to dismiss outdated review"); - } - } - } - } - } - } -} - -#[derive(Debug, Serialize)] -struct FullReview { - pub event: String, - pub body: String, - pub comments: Vec, -} - -#[derive(Debug, Serialize)] -struct ReviewDiffComment { - pub body: String, - pub line: i64, - pub start_line: Option, - pub path: String, -} - -impl From for ReviewDiffComment { - fn from(value: Suggestion) -> Self { - Self { - body: value.suggestion, - line: value.line_end as i64, - start_line: if value.line_end != value.line_start { - Some(value.line_start as i64) - } else { - None - }, - path: value.path, - } - } -} - -/// A structure for deserializing a single changed file in a CI event. -#[derive(Debug, Deserialize, PartialEq, Clone)] -struct GithubChangedFile { - /// The file's name (including relative path to repo root) - pub filename: String, - /// If renamed, this will be the file's old name as a [`Some`], otherwise [`None`]. - pub previous_filename: Option, - /// The individual patch that describes the file's changes. - pub patch: Option, -} - -/// A structure for deserializing a Push event's changed files. -#[derive(Debug, Deserialize, PartialEq, Clone)] -struct PushEventFiles { - /// The list of changed files. - pub files: Vec, -} - -/// A structure for deserializing a comment from a response's json. -#[derive(Debug, Deserialize, PartialEq, Clone)] -struct PullRequestInfo { - /// Is this PR a draft? - pub draft: bool, - /// What is current state of this PR? - /// - /// Here we only care if it is `"open"`. - pub state: String, -} - -/// A structure for deserializing a comment from a response's json. -#[derive(Debug, Deserialize, PartialEq, Clone)] -struct ReviewComment { - /// The content of the review's summary comment. - pub body: Option, - /// The review's ID. - pub id: i64, - /// The state of the review in question. - /// - /// This could be "PENDING", "DISMISSED", "APPROVED", or "COMMENT". - pub state: String, -} - -/// A structure for deserializing a comment from a response's json. -#[derive(Debug, Deserialize, PartialEq, Clone)] -struct ThreadComment { - /// The comment's ID number. - pub id: i64, - /// The comment's body number. - pub body: String, - /// The comment's user number. - /// - /// This is only used for debug output. - pub user: User, -} - -/// A structure for deserializing a comment's author from a response's json. -/// -/// This is only used for debug output. -#[derive(Debug, Deserialize, PartialEq, Clone)] -struct User { - pub login: String, - pub id: u64, -} - -#[cfg(test)] -mod test { - use std::{ - default::Default, - env, - io::Read, - path::PathBuf, - sync::{Arc, Mutex}, - }; - - use chrono::Utc; - use mockito::{Matcher, Server}; - use regex::Regex; - use reqwest::{Method, Url}; - use tempfile::{tempdir, NamedTempFile}; - - use super::GithubApiClient; - use crate::{ - clang_tools::capture_clang_tools_output, - cli::{ClangParams, FeedbackInput, LinesChangedOnly}, - common_fs::FileObj, - rest_api::{RestApiClient, USER_OUTREACH}, - }; - - // ************************* tests for step-summary and output variables - - async fn create_comment(tidy_checks: &str, style: &str) -> (String, String) { - let tmp_dir = tempdir().unwrap(); - let rest_api_client = GithubApiClient::default(); - if env::var("ACTIONS_STEP_DEBUG").is_ok_and(|var| var == "true") { - assert!(rest_api_client.debug_enabled); - } - let mut files = vec![Arc::new(Mutex::new(FileObj::new(PathBuf::from( - "tests/demo/demo.cpp", - ))))]; - let mut clang_params = ClangParams { - tidy_checks: tidy_checks.to_string(), - lines_changed_only: LinesChangedOnly::Off, - style: style.to_string(), - ..Default::default() - }; - capture_clang_tools_output( - &mut files, - env::var("CLANG-VERSION").unwrap_or("".to_string()).as_str(), - &mut clang_params, - ) - .await; - let feedback_inputs = FeedbackInput { - style: style.to_string(), - step_summary: true, - ..Default::default() - }; - let mut step_summary_path = NamedTempFile::new_in(tmp_dir.path()).unwrap(); - env::set_var("GITHUB_STEP_SUMMARY", step_summary_path.path()); - let mut gh_out_path = NamedTempFile::new_in(tmp_dir.path()).unwrap(); - env::set_var("GITHUB_OUTPUT", gh_out_path.path()); - rest_api_client.post_feedback(&files, feedback_inputs).await; - let mut step_summary_content = String::new(); - step_summary_path - .read_to_string(&mut step_summary_content) - .unwrap(); - assert!(&step_summary_content.contains(USER_OUTREACH)); - let mut gh_out_content = String::new(); - gh_out_path.read_to_string(&mut gh_out_content).unwrap(); - assert!(gh_out_content.starts_with("checks-failed=")); - (step_summary_content, gh_out_content) - } - - #[tokio::test] - async fn check_comment_concerns() { - let (comment, gh_out) = create_comment("readability-*", "file").await; - assert!(&comment.contains(":warning:\nSome files did not pass the configured checks!\n")); - let fmt_pattern = Regex::new(r"format-checks-failed=(\d+)\n").unwrap(); - let tidy_pattern = Regex::new(r"tidy-checks-failed=(\d+)\n").unwrap(); - for pattern in [fmt_pattern, tidy_pattern] { - let number = pattern - .captures(&gh_out) - .expect("found no number of checks-failed") - .get(1) - .unwrap() - .as_str() - .parse::() - .unwrap(); - assert!(number > 0); - } - } - - #[tokio::test] - async fn check_comment_lgtm() { - env::set_var("ACTIONS_STEP_DEBUG", "true"); - let (comment, gh_out) = create_comment("-*", "").await; - assert!(&comment.contains(":heavy_check_mark:\nNo problems need attention.")); - assert_eq!( - &gh_out, - "checks-failed=0\nformat-checks-failed=0\ntidy-checks-failed=0\n" - ); - } - - async fn simulate_rate_limit(secondary: bool) { - let mut server = Server::new_async().await; - let url = Url::parse(server.url().as_str()).unwrap(); - env::set_var("GITHUB_API_URL", server.url()); - let client = GithubApiClient::default(); - let reset_timestamp = (Utc::now().timestamp() + 60).to_string(); - let mock = server - .mock("GET", "/") - .match_body(Matcher::Any) - .expect_at_least(1) - .expect_at_most(5) - .with_status(429) - .with_header( - &client.rate_limit_headers.remaining, - if secondary { "1" } else { "0" }, - ) - .with_header(&client.rate_limit_headers.reset, &reset_timestamp); - if secondary { - mock.with_header(&client.rate_limit_headers.retry, "0") - .create(); - } else { - mock.create(); - } - let request = - GithubApiClient::make_api_request(&client.client, url, Method::GET, None, None); - GithubApiClient::send_api_request( - client.client.clone(), - request, - true, - client.rate_limit_headers.clone(), - 0, - ) - .await; - } - - #[tokio::test] - #[ignore] - #[should_panic(expected = "REST API secondary rate limit exceeded")] - async fn secondary_rate_limit() { - simulate_rate_limit(true).await; - } - - #[tokio::test] - #[ignore] - #[should_panic(expected = "REST API rate limit exceeded!")] - async fn primary_rate_limit() { - simulate_rate_limit(false).await; - } -} diff --git a/cpp-linter-py/cpp_linter/__init__.py b/cpp-linter-py/cpp_linter/__init__.py deleted file mode 100644 index 022992d..0000000 --- a/cpp-linter-py/cpp_linter/__init__.py +++ /dev/null @@ -1,8 +0,0 @@ -# type: ignore -# ruff: noqa: F405 F403 -from .cpp_linter import * - - -__doc__ = cpp_linter.__doc__ -if hasattr(cpp_linter, "__all__"): - __all__ = cpp_linter.__all__ diff --git a/cpp-linter-py/cpp_linter/entry_point.py b/cpp-linter-py/cpp_linter/entry_point.py deleted file mode 100644 index 34a71b6..0000000 --- a/cpp-linter-py/cpp_linter/entry_point.py +++ /dev/null @@ -1,29 +0,0 @@ -""" -This module is the python frontend of the cpp-linter package written in Rust. -It exposes a single function: `main()`. - -The idea here is that all functionality is implemented in Rust. However, passing -command line arguments is done differently in Python or Rust. - -- In python, the ``sys.argv`` list is passed from the ``cpp_linter.entry_point.main()`` - function to rust via the ``cpp_linter.run.main()`` binding. -- In rust, the ``std::env::args`` is passed to ``run::run_main()`` in the binary driver - source `main.rs`. - -This is done because of the way the python entry point is invoked. If ``std::env::args`` -is used instead of python's ``sys.argv``, then the list of strings includes the entry -point alias ("path/to/cpp-linter.exe"). Thus, the parser in `cli.rs` will halt on an -error because it is not configured to handle positional arguments. -""" - -import sys - -# Using relative import to load binary lib with same name as root package. -# This is just how pyo3 builds python bindings from rust. -from .cpp_linter import run - - -def main(): - """The main entrypoint for the python frontend. See our rust docs for more info on - the backend (implemented in rust)""" - sys.exit(run.main(sys.argv)) diff --git a/cpp-linter-py/src/lib.rs b/cpp-linter-py/src/lib.rs deleted file mode 100644 index 539c5fc..0000000 --- a/cpp-linter-py/src/lib.rs +++ /dev/null @@ -1,25 +0,0 @@ -use pyo3::prelude::*; -use tokio::runtime::Builder; - -use cpp_linter_lib::run::run_main; - -/// A wrapper for the cpp_linter_lib::run::run_main() -#[pyfunction] -fn main(args: Vec) -> PyResult { - Builder::new_multi_thread() - .enable_all() - .build() - .unwrap() - .block_on(async { Ok(run_main(args).await) }) -} - -/// The python binding for the cpp_linter package. It only exposes a submodule named -/// ``cpp_linter.run`` whose only exposed function is used as the entrypoint script. -/// See the pure python sources in this repo's cpp_linter folder (located at repo root). -#[pymodule] -fn cpp_linter(m: &Bound<'_, PyModule>) -> PyResult<()> { - let run_submodule = PyModule::new_bound(m.py(), "run")?; - run_submodule.add_function(wrap_pyfunction!(main, m)?)?; - m.add_submodule(&run_submodule)?; - Ok(()) -} diff --git a/cpp-linter-lib/.gitignore b/cpp-linter/.gitignore similarity index 100% rename from cpp-linter-lib/.gitignore rename to cpp-linter/.gitignore diff --git a/cpp-linter-lib/Cargo.toml b/cpp-linter/Cargo.toml similarity index 96% rename from cpp-linter-lib/Cargo.toml rename to cpp-linter/Cargo.toml index 2566c65..1bb6f6c 100644 --- a/cpp-linter-lib/Cargo.toml +++ b/cpp-linter/Cargo.toml @@ -1,7 +1,7 @@ [package] -name = "cpp-linter-lib" +name = "cpp-linter" edition = "2021" -repository = "https://github.com/cpp-linter/cpp_linter_rs/tree/main/cpp-linter-lib" +repository = "https://github.com/cpp-linter/cpp_linter_rs/tree/main/cpp-linter" readme = "README.md" version.workspace = true authors.workspace = true diff --git a/cpp-linter-lib/README.md b/cpp-linter/README.md similarity index 100% rename from cpp-linter-lib/README.md rename to cpp-linter/README.md diff --git a/cpp-linter-lib/src/clang_tools/clang_format.rs b/cpp-linter/src/clang_tools/clang_format.rs similarity index 100% rename from cpp-linter-lib/src/clang_tools/clang_format.rs rename to cpp-linter/src/clang_tools/clang_format.rs diff --git a/cpp-linter-lib/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs similarity index 100% rename from cpp-linter-lib/src/clang_tools/clang_tidy.rs rename to cpp-linter/src/clang_tools/clang_tidy.rs diff --git a/cpp-linter-lib/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs similarity index 100% rename from cpp-linter-lib/src/clang_tools/mod.rs rename to cpp-linter/src/clang_tools/mod.rs diff --git a/cpp-linter-lib/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs similarity index 100% rename from cpp-linter-lib/src/cli/mod.rs rename to cpp-linter/src/cli/mod.rs diff --git a/cpp-linter-lib/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs similarity index 100% rename from cpp-linter-lib/src/cli/structs.rs rename to cpp-linter/src/cli/structs.rs diff --git a/cpp-linter-lib/src/common_fs/file_filter.rs b/cpp-linter/src/common_fs/file_filter.rs similarity index 100% rename from cpp-linter-lib/src/common_fs/file_filter.rs rename to cpp-linter/src/common_fs/file_filter.rs diff --git a/cpp-linter-lib/src/common_fs/mod.rs b/cpp-linter/src/common_fs/mod.rs similarity index 99% rename from cpp-linter-lib/src/common_fs/mod.rs rename to cpp-linter/src/common_fs/mod.rs index 72e650f..253d028 100644 --- a/cpp-linter-lib/src/common_fs/mod.rs +++ b/cpp-linter/src/common_fs/mod.rs @@ -279,7 +279,7 @@ mod test { #[test] fn normalize_no_root() { - let src = PathBuf::from("../cpp-linter-lib"); + let src = PathBuf::from("../cpp-linter"); let mut cur_dir = current_dir().unwrap(); cur_dir = cur_dir .strip_prefix(current_dir().unwrap().parent().unwrap()) diff --git a/cpp-linter-lib/src/git.rs b/cpp-linter/src/git.rs similarity index 99% rename from cpp-linter-lib/src/git.rs rename to cpp-linter/src/git.rs index 5c34b92..e98486d 100644 --- a/cpp-linter-lib/src/git.rs +++ b/cpp-linter/src/git.rs @@ -380,7 +380,7 @@ mod test { use crate::{ common_fs::FileFilter, - rest_api::{github_api::GithubApiClient, RestApiClient}, + rest_api::{github::GithubApiClient, RestApiClient}, }; fn get_temp_dir() -> TempDir { diff --git a/cpp-linter-lib/src/lib.rs b/cpp-linter/src/lib.rs similarity index 100% rename from cpp-linter-lib/src/lib.rs rename to cpp-linter/src/lib.rs diff --git a/cpp-linter-lib/src/logger.rs b/cpp-linter/src/logger.rs similarity index 100% rename from cpp-linter-lib/src/logger.rs rename to cpp-linter/src/logger.rs diff --git a/cpp-linter-lib/src/main.rs b/cpp-linter/src/main.rs similarity index 87% rename from cpp-linter-lib/src/main.rs rename to cpp-linter/src/main.rs index 5815e81..cd8ebe8 100644 --- a/cpp-linter-lib/src/main.rs +++ b/cpp-linter/src/main.rs @@ -2,7 +2,7 @@ /// This crate is the binary executable's entrypoint. use std::env; -use cpp_linter_lib::run::run_main; +use ::cpp_linter::run::run_main; /// This function simply forwards CLI args to [`run_main()`]. #[tokio::main] diff --git a/cpp-linter/src/rest_api/github/mod.rs b/cpp-linter/src/rest_api/github/mod.rs new file mode 100644 index 0000000..7d516e9 --- /dev/null +++ b/cpp-linter/src/rest_api/github/mod.rs @@ -0,0 +1,440 @@ +//! This module holds functionality specific to using Github's REST API. +//! +//! In the root module, we just implement the RestApiClient trait. +//! In other (private) submodules we implement behavior specific to Github's REST API. + +use std::env; +use std::fs::OpenOptions; +use std::io::Write; +use std::sync::{Arc, Mutex}; + +// non-std crates +use reqwest::{ + header::{HeaderMap, HeaderValue, AUTHORIZATION}, + Client, Method, Url, +}; +use serde_json; + +// project specific modules/crates +use super::{RestApiClient, RestApiRateLimitHeaders}; +use crate::clang_tools::clang_format::tally_format_advice; +use crate::clang_tools::clang_tidy::tally_tidy_advice; +use crate::cli::{FeedbackInput, ThreadComments}; +use crate::common_fs::{FileFilter, FileObj}; +use crate::git::{get_diff, open_repo, parse_diff, parse_diff_from_buf}; + +// private submodules. +mod serde_structs; +mod specific_api; +use serde_structs::{GithubChangedFile, PushEventFiles}; + +/// A structure to work with Github REST API. +pub struct GithubApiClient { + /// The HTTP request client to be used for all REST API calls. + client: Client, + + /// The CI run's event payload from the webhook that triggered the workflow. + pull_request: Option, + + /// The name of the event that was triggered when running cpp_linter. + pub event_name: String, + + /// The value of the `GITHUB_API_URL` environment variable. + api_url: Url, + + /// The value of the `GITHUB_REPOSITORY` environment variable. + repo: Option, + + /// The value of the `GITHUB_SHA` environment variable. + sha: Option, + + /// The value of the `ACTIONS_STEP_DEBUG` environment variable. + pub debug_enabled: bool, + + rate_limit_headers: RestApiRateLimitHeaders, +} + +// implement the RestApiClient trait for the GithubApiClient +impl RestApiClient for GithubApiClient { + fn set_exit_code( + &self, + checks_failed: u64, + format_checks_failed: Option, + tidy_checks_failed: Option, + ) -> u64 { + if let Ok(gh_out) = env::var("GITHUB_OUTPUT") { + let mut gh_out_file = OpenOptions::new() + .append(true) + .open(gh_out) + .expect("GITHUB_OUTPUT file could not be opened"); + for (prompt, value) in [ + ("checks-failed", Some(checks_failed)), + ("format-checks-failed", format_checks_failed), + ("tidy-checks-failed", tidy_checks_failed), + ] { + if let Err(e) = writeln!(gh_out_file, "{prompt}={}", value.unwrap_or(0),) { + log::error!("Could not write to GITHUB_OUTPUT file: {}", e); + break; + } + } + gh_out_file + .flush() + .expect("Failed to flush buffer to GITHUB_OUTPUT file"); + } + log::info!( + "{} clang-format-checks-failed", + format_checks_failed.unwrap_or(0) + ); + log::info!( + "{} clang-tidy-checks-failed", + tidy_checks_failed.unwrap_or(0) + ); + log::info!("{checks_failed} checks-failed"); + checks_failed + } + + fn make_headers() -> HeaderMap { + let mut headers = HeaderMap::new(); + headers.insert( + "Accept", + HeaderValue::from_str("application/vnd.github.raw+json") + .expect("Failed to create a header value for the API return data type"), + ); + // headers.insert("User-Agent", USER_AGENT.parse().unwrap()); + if let Ok(token) = env::var("GITHUB_TOKEN") { + let mut val = HeaderValue::from_str(token.as_str()) + .expect("Failed to create a secure header value for the API token."); + val.set_sensitive(true); + headers.insert(AUTHORIZATION, val); + } + headers + } + + async fn get_list_of_changed_files(&self, file_filter: &FileFilter) -> Vec { + if env::var("CI").is_ok_and(|val| val.as_str() == "true") + && self.repo.is_some() + && self.sha.is_some() + { + // get diff from Github REST API + let is_pr = self.event_name == "pull_request"; + let pr = self.pull_request.unwrap_or(-1).to_string(); + let sha = self.sha.clone().unwrap(); + let url = self + .api_url + .join("repos/") + .unwrap() + .join(format!("{}/", self.repo.as_ref().unwrap()).as_str()) + .unwrap() + .join(if is_pr { "pulls/" } else { "commits/" }) + .unwrap() + .join(if is_pr { pr.as_str() } else { sha.as_str() }) + .unwrap(); + let mut diff_header = HeaderMap::new(); + diff_header.insert("Accept", "application/vnd.github.diff".parse().unwrap()); + let request = Self::make_api_request( + &self.client, + url.as_str(), + Method::GET, + None, + Some(diff_header), + ); + let response = Self::send_api_request( + self.client.clone(), + request, + false, + self.rate_limit_headers.to_owned(), + 0, + ) + .await; + match response { + Some(response) => { + if response.status.is_success() { + return parse_diff_from_buf(response.text.as_bytes(), file_filter); + } else { + let endpoint = if is_pr { + Url::parse(format!("{}/files", url.as_str()).as_str()) + .expect("failed to parse URL endpoint") + } else { + url + }; + self.get_changed_files_paginated(endpoint, file_filter) + .await + } + } + None => { + panic!("Failed to connect with GitHub server to get list of changed files.") + } + } + } else { + // get diff from libgit2 API + let repo = open_repo(".") + .expect("Please ensure the repository is checked out before running cpp-linter."); + let list = parse_diff(&get_diff(&repo), file_filter); + list + } + } + + async fn get_changed_files_paginated( + &self, + url: Url, + file_filter: &FileFilter, + ) -> Vec { + let mut url = Some(Url::parse_with_params(url.as_str(), &[("page", "1")]).unwrap()); + let mut files = vec![]; + while let Some(ref endpoint) = url { + let request = + Self::make_api_request(&self.client, endpoint.as_str(), Method::GET, None, None); + let response = Self::send_api_request( + self.client.clone(), + request, + true, + self.rate_limit_headers.clone(), + 0, + ) + .await; + if let Some(response) = response { + url = Self::try_next_page(&response.headers); + let files_list = if self.event_name != "pull_request" { + let json_value: PushEventFiles = serde_json::from_str(&response.text) + .expect("Failed to deserialize list of changed files from json response"); + json_value.files + } else { + serde_json::from_str::>(&response.text).expect( + "Failed to deserialize list of file changes from Pull Request event.", + ) + }; + for file in files_list { + if let Some(patch) = file.patch { + let diff = format!( + "diff --git a/{old} b/{new}\n--- a/{old}\n+++ b/{new}\n{patch}", + old = file.previous_filename.unwrap_or(file.filename.clone()), + new = file.filename, + ); + if let Some(file_obj) = + parse_diff_from_buf(diff.as_bytes(), file_filter).first() + { + files.push(file_obj.to_owned()); + } + } + } + } + } + files + } + + async fn post_feedback( + &self, + files: &[Arc>], + feedback_inputs: FeedbackInput, + ) -> u64 { + let tidy_checks_failed = tally_tidy_advice(files); + let format_checks_failed = tally_format_advice(files); + let mut comment = None; + + if feedback_inputs.file_annotations { + self.post_annotations(files, feedback_inputs.style.as_str()); + } + if feedback_inputs.step_summary { + comment = + Some(self.make_comment(files, format_checks_failed, tidy_checks_failed, None)); + self.post_step_summary(comment.as_ref().unwrap()); + } + self.set_exit_code( + format_checks_failed + tidy_checks_failed, + Some(format_checks_failed), + Some(tidy_checks_failed), + ); + + if feedback_inputs.thread_comments != ThreadComments::Off { + // post thread comment for PR or push event + if comment.as_ref().is_some_and(|c| c.len() > 65535) || comment.is_none() { + comment = Some(self.make_comment( + files, + format_checks_failed, + tidy_checks_failed, + Some(65535), + )); + } + if let Some(repo) = &self.repo { + let is_pr = self.event_name == "pull_request"; + let pr = self.pull_request.unwrap_or(-1).to_string() + "/"; + let sha = self.sha.clone().unwrap() + "/"; + let comments_url = self + .api_url + .join("repos/") + .unwrap() + .join(format!("{}/", repo).as_str()) + .unwrap() + .join(if is_pr { "issues/" } else { "commits/" }) + .unwrap() + .join(if is_pr { pr.as_str() } else { sha.as_str() }) + .unwrap() + .join("comments/") + .unwrap(); + + self.update_comment( + comments_url, + &comment.unwrap(), + feedback_inputs.no_lgtm, + format_checks_failed + tidy_checks_failed == 0, + feedback_inputs.thread_comments == ThreadComments::Update, + ) + .await; + } + } + if self.event_name == "pull_request" + && (feedback_inputs.tidy_review || feedback_inputs.format_review) + { + self.post_review(files, &feedback_inputs).await; + } + format_checks_failed + tidy_checks_failed + } +} + +#[cfg(test)] +mod test { + use std::{ + default::Default, + env, + io::Read, + path::PathBuf, + sync::{Arc, Mutex}, + }; + + use chrono::Utc; + use mockito::{Matcher, Server}; + use regex::Regex; + use reqwest::{Method, Url}; + use tempfile::{tempdir, NamedTempFile}; + + use super::GithubApiClient; + use crate::{ + clang_tools::capture_clang_tools_output, + cli::{ClangParams, FeedbackInput, LinesChangedOnly}, + common_fs::FileObj, + rest_api::{RestApiClient, USER_OUTREACH}, + }; + + // ************************* tests for step-summary and output variables + + async fn create_comment(tidy_checks: &str, style: &str) -> (String, String) { + let tmp_dir = tempdir().unwrap(); + let rest_api_client = GithubApiClient::default(); + if env::var("ACTIONS_STEP_DEBUG").is_ok_and(|var| var == "true") { + assert!(rest_api_client.debug_enabled); + } + let mut files = vec![Arc::new(Mutex::new(FileObj::new(PathBuf::from( + "tests/demo/demo.cpp", + ))))]; + let mut clang_params = ClangParams { + tidy_checks: tidy_checks.to_string(), + lines_changed_only: LinesChangedOnly::Off, + style: style.to_string(), + ..Default::default() + }; + capture_clang_tools_output( + &mut files, + env::var("CLANG-VERSION").unwrap_or("".to_string()).as_str(), + &mut clang_params, + ) + .await; + let feedback_inputs = FeedbackInput { + style: style.to_string(), + step_summary: true, + ..Default::default() + }; + let mut step_summary_path = NamedTempFile::new_in(tmp_dir.path()).unwrap(); + env::set_var("GITHUB_STEP_SUMMARY", step_summary_path.path()); + let mut gh_out_path = NamedTempFile::new_in(tmp_dir.path()).unwrap(); + env::set_var("GITHUB_OUTPUT", gh_out_path.path()); + rest_api_client.post_feedback(&files, feedback_inputs).await; + let mut step_summary_content = String::new(); + step_summary_path + .read_to_string(&mut step_summary_content) + .unwrap(); + assert!(&step_summary_content.contains(USER_OUTREACH)); + let mut gh_out_content = String::new(); + gh_out_path.read_to_string(&mut gh_out_content).unwrap(); + assert!(gh_out_content.starts_with("checks-failed=")); + (step_summary_content, gh_out_content) + } + + #[tokio::test] + async fn check_comment_concerns() { + let (comment, gh_out) = create_comment("readability-*", "file").await; + assert!(&comment.contains(":warning:\nSome files did not pass the configured checks!\n")); + let fmt_pattern = Regex::new(r"format-checks-failed=(\d+)\n").unwrap(); + let tidy_pattern = Regex::new(r"tidy-checks-failed=(\d+)\n").unwrap(); + for pattern in [fmt_pattern, tidy_pattern] { + let number = pattern + .captures(&gh_out) + .expect("found no number of checks-failed") + .get(1) + .unwrap() + .as_str() + .parse::() + .unwrap(); + assert!(number > 0); + } + } + + #[tokio::test] + async fn check_comment_lgtm() { + env::set_var("ACTIONS_STEP_DEBUG", "true"); + let (comment, gh_out) = create_comment("-*", "").await; + assert!(&comment.contains(":heavy_check_mark:\nNo problems need attention.")); + assert_eq!( + &gh_out, + "checks-failed=0\nformat-checks-failed=0\ntidy-checks-failed=0\n" + ); + } + + async fn simulate_rate_limit(secondary: bool) { + let mut server = Server::new_async().await; + let url = Url::parse(server.url().as_str()).unwrap(); + env::set_var("GITHUB_API_URL", server.url()); + let client = GithubApiClient::default(); + let reset_timestamp = (Utc::now().timestamp() + 60).to_string(); + let mock = server + .mock("GET", "/") + .match_body(Matcher::Any) + .expect_at_least(1) + .expect_at_most(5) + .with_status(429) + .with_header( + &client.rate_limit_headers.remaining, + if secondary { "1" } else { "0" }, + ) + .with_header(&client.rate_limit_headers.reset, &reset_timestamp); + if secondary { + mock.with_header(&client.rate_limit_headers.retry, "0") + .create(); + } else { + mock.create(); + } + let request = + GithubApiClient::make_api_request(&client.client, url, Method::GET, None, None); + GithubApiClient::send_api_request( + client.client.clone(), + request, + true, + client.rate_limit_headers.clone(), + 0, + ) + .await; + } + + #[tokio::test] + #[ignore] + #[should_panic(expected = "REST API secondary rate limit exceeded")] + async fn secondary_rate_limit() { + simulate_rate_limit(true).await; + } + + #[tokio::test] + #[ignore] + #[should_panic(expected = "REST API rate limit exceeded!")] + async fn primary_rate_limit() { + simulate_rate_limit(false).await; + } +} diff --git a/cpp-linter/src/rest_api/github/serde_structs.rs b/cpp-linter/src/rest_api/github/serde_structs.rs new file mode 100644 index 0000000..df03443 --- /dev/null +++ b/cpp-linter/src/rest_api/github/serde_structs.rs @@ -0,0 +1,100 @@ +//! This submodule declares data structures used to +//! deserialize (and serializer) JSON payload data. + +use serde::{Deserialize, Serialize}; + +use crate::clang_tools::Suggestion; + +#[derive(Debug, Serialize)] +pub struct FullReview { + pub event: String, + pub body: String, + pub comments: Vec, +} + +#[derive(Debug, Serialize)] +pub struct ReviewDiffComment { + pub body: String, + pub line: i64, + pub start_line: Option, + pub path: String, +} + +impl From for ReviewDiffComment { + fn from(value: Suggestion) -> Self { + Self { + body: value.suggestion, + line: value.line_end as i64, + start_line: if value.line_end != value.line_start { + Some(value.line_start as i64) + } else { + None + }, + path: value.path, + } + } +} + +/// A structure for deserializing a single changed file in a CI event. +#[derive(Debug, Deserialize, PartialEq, Clone)] +pub struct GithubChangedFile { + /// The file's name (including relative path to repo root) + pub filename: String, + /// If renamed, this will be the file's old name as a [`Some`], otherwise [`None`]. + pub previous_filename: Option, + /// The individual patch that describes the file's changes. + pub patch: Option, +} + +/// A structure for deserializing a Push event's changed files. +#[derive(Debug, Deserialize, PartialEq, Clone)] +pub struct PushEventFiles { + /// The list of changed files. + pub files: Vec, +} + +/// A structure for deserializing a comment from a response's json. +#[derive(Debug, Deserialize, PartialEq, Clone)] +pub struct PullRequestInfo { + /// Is this PR a draft? + pub draft: bool, + /// What is current state of this PR? + /// + /// Here we only care if it is `"open"`. + pub state: String, +} + +/// A structure for deserializing a comment from a response's json. +#[derive(Debug, Deserialize, PartialEq, Clone)] +pub struct ReviewComment { + /// The content of the review's summary comment. + pub body: Option, + /// The review's ID. + pub id: i64, + /// The state of the review in question. + /// + /// This could be "PENDING", "DISMISSED", "APPROVED", or "COMMENT". + pub state: String, +} + +/// A structure for deserializing a comment from a response's json. +#[derive(Debug, Deserialize, PartialEq, Clone)] +pub struct ThreadComment { + /// The comment's ID number. + pub id: i64, + /// The comment's body number. + pub body: String, + /// The comment's user number. + /// + /// This is only used for debug output. + pub user: User, +} + +/// A structure for deserializing a comment's author from a response's json. +/// +/// This is only used for debug output. +#[derive(Debug, Deserialize, PartialEq, Clone)] +pub struct User { + pub login: String, + pub id: u64, +} diff --git a/cpp-linter/src/rest_api/github/specific_api.rs b/cpp-linter/src/rest_api/github/specific_api.rs new file mode 100644 index 0000000..4d1abb0 --- /dev/null +++ b/cpp-linter/src/rest_api/github/specific_api.rs @@ -0,0 +1,448 @@ +//! This submodule implements functionality exclusively specific to Github's REST API. + +use std::{ + collections::HashMap, + env, + fs::OpenOptions, + io::{Read, Write}, + sync::{Arc, Mutex}, +}; + +use reqwest::{Client, Method, Url}; + +use crate::{ + clang_tools::{clang_format::summarize_style, ReviewComments}, + cli::FeedbackInput, + common_fs::FileObj, + rest_api::{RestApiRateLimitHeaders, COMMENT_MARKER}, +}; + +use super::{ + serde_structs::{FullReview, PullRequestInfo, ReviewComment, ReviewDiffComment, ThreadComment}, + GithubApiClient, RestApiClient, +}; + +impl Default for GithubApiClient { + fn default() -> Self { + Self::new() + } +} + +impl GithubApiClient { + /// Instantiate a [`GithubApiClient`] object. + pub fn new() -> Self { + let event_name = env::var("GITHUB_EVENT_NAME").unwrap_or(String::from("unknown")); + let pull_request = { + match event_name.as_str() { + "pull_request" => { + let event_payload_path = env::var("GITHUB_EVENT_PATH") + .expect("GITHUB_EVENT_NAME is set to 'pull_request', but GITHUB_EVENT_PATH is not set"); + let file_buf = &mut String::new(); + OpenOptions::new() + .read(true) + .open(event_payload_path) + .unwrap() + .read_to_string(file_buf) + .unwrap(); + let json = serde_json::from_str::>( + file_buf, + ) + .unwrap(); + json["number"].as_i64() + } + _ => None, + } + }; + let api_url = Url::parse( + env::var("GITHUB_API_URL") + .unwrap_or("https://api.github.com".to_string()) + .as_str(), + ) + .expect("Failed to parse URL from GITHUB_API_URL"); + + GithubApiClient { + client: Client::builder() + .default_headers(Self::make_headers()) + .build() + .expect("Failed to create a session client for REST API calls"), + pull_request, + event_name, + api_url, + repo: match env::var("GITHUB_REPOSITORY") { + Ok(val) => Some(val), + Err(_) => None, + }, + sha: match env::var("GITHUB_SHA") { + Ok(val) => Some(val), + Err(_) => None, + }, + debug_enabled: match env::var("ACTIONS_STEP_DEBUG") { + Ok(val) => val == "true", + Err(_) => false, + }, + rate_limit_headers: RestApiRateLimitHeaders { + reset: "x-ratelimit-reset".to_string(), + remaining: "x-ratelimit-remaining".to_string(), + retry: "retry-after".to_string(), + }, + } + } + + /// Append step summary to CI workflow's summary page. + pub fn post_step_summary(&self, comment: &String) { + if let Ok(gh_out) = env::var("GITHUB_STEP_SUMMARY") { + let mut gh_out_file = OpenOptions::new() + .append(true) + .open(gh_out) + .expect("GITHUB_STEP_SUMMARY file could not be opened"); + if let Err(e) = writeln!(gh_out_file, "\n{}\n", comment) { + log::error!("Could not write to GITHUB_STEP_SUMMARY file: {}", e); + } + } + } + + /// Post file annotations. + pub fn post_annotations(&self, files: &[Arc>], style: &str) { + let style_guide = summarize_style(style); + + // iterate over clang-format advice and post annotations + for file in files { + let file = file.lock().unwrap(); + if let Some(format_advice) = &file.format_advice { + // assemble a list of line numbers + let mut lines: Vec = Vec::new(); + for replacement in &format_advice.replacements { + if let Some(line_int) = replacement.line { + if !lines.contains(&line_int) { + lines.push(line_int); + } + } + } + // post annotation if any applicable lines were formatted + if !lines.is_empty() { + println!( + "::notice file={name},title=Run clang-format on {name}::File {name} does not conform to {style_guide} style guidelines. (lines {line_set})", + name = &file.name.to_string_lossy().replace('\\', "/"), + line_set = lines.iter().map(|val| val.to_string()).collect::>().join(","), + ); + } + } // end format_advice iterations + + // iterate over clang-tidy advice and post annotations + // The tidy_advice vector is parallel to the files vector; meaning it serves as a file filterer. + // lines are already filter as specified to clang-tidy CLI. + if let Some(tidy_advice) = &file.tidy_advice { + for note in &tidy_advice.notes { + if note.filename == file.name.to_string_lossy().replace('\\', "/") { + println!( + "::{severity} file={file},line={line},title={file}:{line}:{cols} [{diag}]::{info}", + severity = if note.severity == *"note" { "notice".to_string() } else {note.severity.clone()}, + file = note.filename, + line = note.line, + cols = note.cols, + diag = note.diagnostic, + info = note.rationale, + ); + } + } + } + } + } + + /// Update existing comment or remove old comment(s) and post a new comment + pub async fn update_comment( + &self, + url: Url, + comment: &String, + no_lgtm: bool, + is_lgtm: bool, + update_only: bool, + ) { + let comment_url = self + .remove_bot_comments(&url, !update_only || (is_lgtm && no_lgtm)) + .await; + #[allow(clippy::nonminimal_bool)] // an inaccurate assessment + if (is_lgtm && !no_lgtm) || !is_lgtm { + let payload = HashMap::from([("body", comment.to_owned())]); + // log::debug!("payload body:\n{:?}", payload); + let req_meth = if comment_url.is_some() { + Method::PATCH + } else { + Method::POST + }; + let request = Self::make_api_request( + &self.client, + if let Some(url_) = comment_url { + url_ + } else { + url + }, + req_meth, + Some( + serde_json::to_string(&payload) + .expect("Failed to serialize thread comment to json string"), + ), + None, + ); + Self::send_api_request( + self.client.clone(), + request, + false, + self.rate_limit_headers.to_owned(), + 0, + ) + .await; + } + } + + /// Remove thread comments previously posted by cpp-linter. + async fn remove_bot_comments(&self, url: &Url, delete: bool) -> Option { + let mut comment_url = None; + let mut comments_url = Some( + Url::parse_with_params(url.as_str(), &[("page", "1")]) + .expect("Failed to parse invalid URL string"), + ); + let repo = format!( + "repos/{}/comments/", + self.repo.as_ref().expect("Repo name unknown.") + ); + let base_comment_url = self.api_url.join(&repo).unwrap(); + while let Some(ref endpoint) = comments_url { + let request = + Self::make_api_request(&self.client, endpoint.as_str(), Method::GET, None, None); + let response = Self::send_api_request( + self.client.clone(), + request, + false, + self.rate_limit_headers.to_owned(), + 0, + ) + .await; + if response.is_none() || response.as_ref().is_some_and(|r| !r.status.is_success()) { + log::error!("Failed to get list of existing comments from {}", endpoint); + return comment_url; + } + comments_url = Self::try_next_page(&response.as_ref().unwrap().headers); + let payload: Vec = serde_json::from_str(&response.unwrap().text) + .expect("Failed to serialize response's text"); + for comment in payload { + if comment.body.starts_with(COMMENT_MARKER) { + log::debug!( + "comment id {} from user {} ({})", + comment.id, + comment.user.login, + comment.user.id, + ); + #[allow(clippy::nonminimal_bool)] // an inaccurate assessment + if delete || (!delete && comment_url.is_some()) { + // if not updating: remove all outdated comments + // if updating: remove all outdated comments except the last one + + // use last saved comment_url (if not None) or current comment url + let del_url = if let Some(last_url) = &comment_url { + last_url + } else { + let comment_id = comment.id.to_string(); + &base_comment_url + .join(&comment_id) + .expect("Failed to parse URL from JSON comment.url") + }; + let req = Self::make_api_request( + &self.client, + del_url.clone(), + Method::DELETE, + None, + None, + ); + Self::send_api_request( + self.client.clone(), + req, + false, + self.rate_limit_headers.to_owned(), + 0, + ) + .await; + } + if !delete { + let comment_id = comment.id.to_string(); + comment_url = Some( + base_comment_url + .join(&comment_id) + .expect("Failed to parse URL from JSON comment.url"), + ) + } + } + } + } + comment_url + } + + /// Post a PR review with code suggestions. + /// + /// Note: `--no-lgtm` is applied when nothing is suggested. + pub async fn post_review(&self, files: &[Arc>], feedback_input: &FeedbackInput) { + let url = self + .api_url + .join("repos/") + .unwrap() + .join(format!("{}/", self.repo.as_ref().expect("Repo name unknown")).as_str()) + .unwrap() + .join("pulls/") + .unwrap() + .join( + self.pull_request + .expect("pull request number unknown") + .to_string() + .as_str(), + ) + .unwrap(); + let request = Self::make_api_request(&self.client, url.as_str(), Method::GET, None, None); + let response = Self::send_api_request( + self.client.clone(), + request, + true, + self.rate_limit_headers.clone(), + 0, + ) + .await; + let pr_info: PullRequestInfo = + serde_json::from_str(&response.expect("Failed to get PR info").text) + .expect("Failed to deserialize PR info"); + + let url = Url::parse(format!("{}/", url.as_str()).as_str()) + .unwrap() + .join("reviews") + .expect("Failed to parse URL endpoint for PR reviews"); + let dismissal = self.dismiss_outdated_reviews(&url); + + if pr_info.draft || pr_info.state != "open" { + dismissal.await; + return; + } + + let summary_only = + env::var("CPP_LINTER_PR_REVIEW_SUMMARY_ONLY").unwrap_or("false".to_string()) == "true"; + + let mut review_comments = ReviewComments::default(); + for file in files { + let file = file.lock().unwrap(); + file.make_suggestions_from_patch(&mut review_comments, summary_only); + } + let has_no_changes = + review_comments.full_patch[0].is_empty() && review_comments.full_patch[1].is_empty(); + if has_no_changes && feedback_input.no_lgtm { + log::debug!("Not posting an approved review because `no-lgtm` is true"); + dismissal.await; + return; + } + let mut payload = FullReview { + event: if feedback_input.passive_reviews { + String::from("COMMENT") + } else if has_no_changes { + String::from("APPROVE") + } else { + String::from("REQUEST_CHANGES") + }, + body: String::new(), + comments: vec![], + }; + payload.body = review_comments.summarize(); + if !summary_only { + payload.comments = { + let mut comments = vec![]; + for comment in review_comments.comments { + comments.push(ReviewDiffComment::from(comment)); + } + comments + }; + } + dismissal.await; // free up the `url` variable + let request = Self::make_api_request( + &self.client, + url, + Method::POST, + Some( + serde_json::to_string(&payload) + .expect("Failed to serialize PR review to json string"), + ), + None, + ); + let response = Self::send_api_request( + self.client.clone(), + request, + false, + self.rate_limit_headers.clone(), + 0, + ) + .await; + if response.is_none() || response.is_some_and(|r| !r.status.is_success()) { + log::error!("Failed to post a new PR review"); + } + } + + /// Dismiss any outdated reviews generated by cpp-linter. + async fn dismiss_outdated_reviews(&self, url: &Url) { + let mut url_ = Some( + Url::parse_with_params(url.as_str(), [("page", "1")]) + .expect("Failed to parse endpoint for getting existing PR reviews"), + ); + while let Some(ref endpoint) = url_ { + let request = + Self::make_api_request(&self.client, endpoint.as_str(), Method::GET, None, None); + let response = Self::send_api_request( + self.client.clone(), + request, + false, + self.rate_limit_headers.clone(), + 0, + ) + .await; + if response.is_none() || response.as_ref().is_some_and(|r| !r.status.is_success()) { + log::error!("Failed to get a list of existing PR reviews"); + return; + } + let response = response.unwrap(); + url_ = Self::try_next_page(&response.headers); + let payload: Vec = serde_json::from_str(&response.text) + .expect("Unable to deserialize malformed JSON about review comments"); + for review in payload { + if let Some(body) = &review.body { + if body.starts_with(COMMENT_MARKER) + && !(["PENDING", "DISMISSED"].contains(&review.state.as_str())) + { + // dismiss outdated review + let req = Self::make_api_request( + &self.client, + url.join("reviews/") + .unwrap() + .join(review.id.to_string().as_str()) + .expect("Failed to parse URL for dismissing outdated review."), + Method::PUT, + Some( + serde_json::json!( + { + "message": "outdated suggestion", + "event": "DISMISS" + } + ) + .to_string(), + ), + None, + ); + let result = Self::send_api_request( + self.client.clone(), + req, + false, + self.rate_limit_headers.clone(), + 0, + ) + .await; + if result.is_none() || result.is_some_and(|r| !r.status.is_success()) { + log::error!("Failed to dismiss outdated review"); + } + } + } + } + } + } +} diff --git a/cpp-linter-lib/src/rest_api/mod.rs b/cpp-linter/src/rest_api/mod.rs similarity index 99% rename from cpp-linter-lib/src/rest_api/mod.rs rename to cpp-linter/src/rest_api/mod.rs index 505bd5a..b0059cd 100644 --- a/cpp-linter-lib/src/rest_api/mod.rs +++ b/cpp-linter/src/rest_api/mod.rs @@ -1,4 +1,4 @@ -//! This crate is the home of functionality that uses the REST API of various git-based +//! This module is the home of functionality that uses the REST API of various git-based //! servers. //! //! Currently, only Github is supported. @@ -14,8 +14,8 @@ use futures::future::{BoxFuture, FutureExt}; use reqwest::header::{HeaderMap, HeaderValue}; use reqwest::{Client, IntoUrl, Method, Request, Response, StatusCode, Url}; -// project specific modules/crates -pub mod github_api; +// project specific modules +pub mod github; use crate::cli::FeedbackInput; use crate::common_fs::{FileFilter, FileObj}; diff --git a/cpp-linter-lib/src/run.rs b/cpp-linter/src/run.rs similarity index 99% rename from cpp-linter-lib/src/run.rs rename to cpp-linter/src/run.rs index e9ef6aa..c2bcb0c 100644 --- a/cpp-linter-lib/src/run.rs +++ b/cpp-linter/src/run.rs @@ -17,7 +17,7 @@ use crate::clang_tools::capture_clang_tools_output; use crate::cli::{get_arg_parser, ClangParams, Cli, FeedbackInput, LinesChangedOnly}; use crate::common_fs::FileFilter; use crate::logger::{self, end_log_group, start_log_group}; -use crate::rest_api::{github_api::GithubApiClient, RestApiClient}; +use crate::rest_api::{github::GithubApiClient, RestApiClient}; const VERSION: &str = env!("CARGO_PKG_VERSION"); diff --git a/cpp-linter-lib/tests/.hidden/test_asset.txt b/cpp-linter/tests/.hidden/test_asset.txt similarity index 100% rename from cpp-linter-lib/tests/.hidden/test_asset.txt rename to cpp-linter/tests/.hidden/test_asset.txt diff --git a/cpp-linter-lib/tests/comment_test_assets/patch.diff b/cpp-linter/tests/comment_test_assets/patch.diff similarity index 100% rename from cpp-linter-lib/tests/comment_test_assets/patch.diff rename to cpp-linter/tests/comment_test_assets/patch.diff diff --git a/cpp-linter-lib/tests/comment_test_assets/pr_comments_pg1.json b/cpp-linter/tests/comment_test_assets/pr_comments_pg1.json similarity index 100% rename from cpp-linter-lib/tests/comment_test_assets/pr_comments_pg1.json rename to cpp-linter/tests/comment_test_assets/pr_comments_pg1.json diff --git a/cpp-linter-lib/tests/comment_test_assets/pr_comments_pg2.json b/cpp-linter/tests/comment_test_assets/pr_comments_pg2.json similarity index 100% rename from cpp-linter-lib/tests/comment_test_assets/pr_comments_pg2.json rename to cpp-linter/tests/comment_test_assets/pr_comments_pg2.json diff --git a/cpp-linter-lib/tests/comment_test_assets/push_comments_8d68756375e0483c7ac2b4d6bbbece420dbbb495.json b/cpp-linter/tests/comment_test_assets/push_comments_8d68756375e0483c7ac2b4d6bbbece420dbbb495.json similarity index 100% rename from cpp-linter-lib/tests/comment_test_assets/push_comments_8d68756375e0483c7ac2b4d6bbbece420dbbb495.json rename to cpp-linter/tests/comment_test_assets/push_comments_8d68756375e0483c7ac2b4d6bbbece420dbbb495.json diff --git a/cpp-linter-lib/tests/comments.rs b/cpp-linter/tests/comments.rs similarity index 99% rename from cpp-linter-lib/tests/comments.rs rename to cpp-linter/tests/comments.rs index 9970e7a..e4856b9 100644 --- a/cpp-linter-lib/tests/comments.rs +++ b/cpp-linter/tests/comments.rs @@ -1,6 +1,6 @@ use chrono::Utc; -use cpp_linter_lib::cli::{LinesChangedOnly, ThreadComments}; -use cpp_linter_lib::run::run_main; +use cpp_linter::cli::{LinesChangedOnly, ThreadComments}; +use cpp_linter::run::run_main; use mockito::Matcher; use std::{env, fmt::Display, io::Write, path::Path}; use tempfile::NamedTempFile; diff --git a/cpp-linter-lib/tests/common/mod.rs b/cpp-linter/tests/common/mod.rs similarity index 100% rename from cpp-linter-lib/tests/common/mod.rs rename to cpp-linter/tests/common/mod.rs diff --git a/cpp-linter-lib/tests/demo/.clang-format b/cpp-linter/tests/demo/.clang-format similarity index 100% rename from cpp-linter-lib/tests/demo/.clang-format rename to cpp-linter/tests/demo/.clang-format diff --git a/cpp-linter-lib/tests/demo/.clang-tidy b/cpp-linter/tests/demo/.clang-tidy similarity index 100% rename from cpp-linter-lib/tests/demo/.clang-tidy rename to cpp-linter/tests/demo/.clang-tidy diff --git a/cpp-linter-lib/tests/demo/compile_flags.txt b/cpp-linter/tests/demo/compile_flags.txt similarity index 100% rename from cpp-linter-lib/tests/demo/compile_flags.txt rename to cpp-linter/tests/demo/compile_flags.txt diff --git a/cpp-linter-lib/tests/demo/demo.cpp b/cpp-linter/tests/demo/demo.cpp similarity index 100% rename from cpp-linter-lib/tests/demo/demo.cpp rename to cpp-linter/tests/demo/demo.cpp diff --git a/cpp-linter-lib/tests/demo/demo.hpp b/cpp-linter/tests/demo/demo.hpp similarity index 100% rename from cpp-linter-lib/tests/demo/demo.hpp rename to cpp-linter/tests/demo/demo.hpp diff --git a/cpp-linter-lib/tests/demo/some source.c b/cpp-linter/tests/demo/some source.c similarity index 100% rename from cpp-linter-lib/tests/demo/some source.c rename to cpp-linter/tests/demo/some source.c diff --git a/cpp-linter-lib/tests/git_status_test_assets/cpp-linter/cpp-linter/950ff0b690e1903797c303c5fc8d9f3b52f1d3c5.diff b/cpp-linter/tests/git_status_test_assets/cpp-linter/cpp-linter/950ff0b690e1903797c303c5fc8d9f3b52f1d3c5.diff similarity index 100% rename from cpp-linter-lib/tests/git_status_test_assets/cpp-linter/cpp-linter/950ff0b690e1903797c303c5fc8d9f3b52f1d3c5.diff rename to cpp-linter/tests/git_status_test_assets/cpp-linter/cpp-linter/950ff0b690e1903797c303c5fc8d9f3b52f1d3c5.diff diff --git a/cpp-linter-lib/tests/git_status_test_assets/cpp-linter/cpp-linter/test_git_lib.patch b/cpp-linter/tests/git_status_test_assets/cpp-linter/cpp-linter/test_git_lib.patch similarity index 100% rename from cpp-linter-lib/tests/git_status_test_assets/cpp-linter/cpp-linter/test_git_lib.patch rename to cpp-linter/tests/git_status_test_assets/cpp-linter/cpp-linter/test_git_lib.patch diff --git a/cpp-linter-lib/tests/ignored_paths/.gitmodules b/cpp-linter/tests/ignored_paths/.gitmodules similarity index 100% rename from cpp-linter-lib/tests/ignored_paths/.gitmodules rename to cpp-linter/tests/ignored_paths/.gitmodules diff --git a/cpp-linter-lib/tests/paginated_changed_files.rs b/cpp-linter/tests/paginated_changed_files.rs similarity index 98% rename from cpp-linter-lib/tests/paginated_changed_files.rs rename to cpp-linter/tests/paginated_changed_files.rs index f39bb36..32d8abd 100644 --- a/cpp-linter-lib/tests/paginated_changed_files.rs +++ b/cpp-linter/tests/paginated_changed_files.rs @@ -4,9 +4,9 @@ use common::{create_test_space, mock_server}; use mockito::Matcher; use tempfile::{NamedTempFile, TempDir}; -use cpp_linter_lib::{ +use cpp_linter::{ common_fs::FileFilter, - rest_api::{github_api::GithubApiClient, RestApiClient}, + rest_api::{github::GithubApiClient, RestApiClient}, }; use std::{env, io::Write, path::Path}; diff --git a/cpp-linter-lib/tests/paginated_changes/pull_request_files_pg1.json b/cpp-linter/tests/paginated_changes/pull_request_files_pg1.json similarity index 100% rename from cpp-linter-lib/tests/paginated_changes/pull_request_files_pg1.json rename to cpp-linter/tests/paginated_changes/pull_request_files_pg1.json diff --git a/cpp-linter-lib/tests/paginated_changes/pull_request_files_pg2.json b/cpp-linter/tests/paginated_changes/pull_request_files_pg2.json similarity index 100% rename from cpp-linter-lib/tests/paginated_changes/pull_request_files_pg2.json rename to cpp-linter/tests/paginated_changes/pull_request_files_pg2.json diff --git a/cpp-linter-lib/tests/paginated_changes/push_files_pg1.json b/cpp-linter/tests/paginated_changes/push_files_pg1.json similarity index 100% rename from cpp-linter-lib/tests/paginated_changes/push_files_pg1.json rename to cpp-linter/tests/paginated_changes/push_files_pg1.json diff --git a/cpp-linter-lib/tests/paginated_changes/push_files_pg2.json b/cpp-linter/tests/paginated_changes/push_files_pg2.json similarity index 100% rename from cpp-linter-lib/tests/paginated_changes/push_files_pg2.json rename to cpp-linter/tests/paginated_changes/push_files_pg2.json diff --git a/cpp-linter-lib/tests/reviews.rs b/cpp-linter/tests/reviews.rs similarity index 99% rename from cpp-linter-lib/tests/reviews.rs rename to cpp-linter/tests/reviews.rs index 4f27018..9d8a2fe 100644 --- a/cpp-linter-lib/tests/reviews.rs +++ b/cpp-linter/tests/reviews.rs @@ -1,5 +1,5 @@ use chrono::Utc; -use cpp_linter_lib::{ +use cpp_linter::{ cli::LinesChangedOnly, rest_api::{COMMENT_MARKER, USER_OUTREACH}, run::run_main, diff --git a/cpp-linter-lib/tests/reviews_test_assets/.clang-format b/cpp-linter/tests/reviews_test_assets/.clang-format similarity index 100% rename from cpp-linter-lib/tests/reviews_test_assets/.clang-format rename to cpp-linter/tests/reviews_test_assets/.clang-format diff --git a/cpp-linter-lib/tests/reviews_test_assets/.clang-tidy b/cpp-linter/tests/reviews_test_assets/.clang-tidy similarity index 100% rename from cpp-linter-lib/tests/reviews_test_assets/.clang-tidy rename to cpp-linter/tests/reviews_test_assets/.clang-tidy diff --git a/cpp-linter-lib/tests/reviews_test_assets/pr_27.diff b/cpp-linter/tests/reviews_test_assets/pr_27.diff similarity index 100% rename from cpp-linter-lib/tests/reviews_test_assets/pr_27.diff rename to cpp-linter/tests/reviews_test_assets/pr_27.diff diff --git a/cpp-linter-lib/tests/reviews_test_assets/pr_review_comments.json b/cpp-linter/tests/reviews_test_assets/pr_review_comments.json similarity index 100% rename from cpp-linter-lib/tests/reviews_test_assets/pr_review_comments.json rename to cpp-linter/tests/reviews_test_assets/pr_review_comments.json diff --git a/cpp-linter-lib/tests/reviews_test_assets/pr_reviews.json b/cpp-linter/tests/reviews_test_assets/pr_reviews.json similarity index 100% rename from cpp-linter-lib/tests/reviews_test_assets/pr_reviews.json rename to cpp-linter/tests/reviews_test_assets/pr_reviews.json diff --git a/docs/Cargo.toml b/docs/Cargo.toml index ac7be34..10d34dc 100644 --- a/docs/Cargo.toml +++ b/docs/Cargo.toml @@ -9,7 +9,7 @@ homepage.workspace = true license.workspace = true [dependencies] -cpp-linter-lib = { path = "../cpp-linter-lib" } +cpp-linter = { path = "../cpp-linter" } clap = "4.5.17" mdbook = "0.4.40" semver = "1.0.23" diff --git a/docs/src/main.rs b/docs/src/main.rs index 6a1ce24..a0350db 100644 --- a/docs/src/main.rs +++ b/docs/src/main.rs @@ -12,7 +12,7 @@ use semver::{Version, VersionReq}; use std::io; use std::process; -extern crate cpp_linter_lib; +extern crate cpp_linter; use cli_gen_lib::CliGen; @@ -82,7 +82,7 @@ mod cli_gen_lib { use mdbook::book::Book; use mdbook::preprocess::{Preprocessor, PreprocessorContext}; - use cpp_linter_lib::cli; + use cpp_linter::cli; pub struct CliGen; diff --git a/justfile b/justfile index f8d7c61..1d0cfef 100644 --- a/justfile +++ b/justfile @@ -1,27 +1,10 @@ set windows-shell := ["powershell.exe", "-NoLogo", "-Command"] -# activate python venv -[group("python")] -[windows] -venv: - ./.env/Scripts/activate - -# activate python venv -[group("python")] -[linux] -venv: - . ./.env/bin/activate - -# install python bindings -[group("python")] -py-dev: - maturin dev --manifest-path cpp-linter-py/Cargo.toml - # run the test suite [group("code coverage")] test arg='': cargo llvm-cov --no-report \ - nextest --manifest-path cpp-linter-lib/Cargo.toml \ + nextest --manifest-path cpp-linter/Cargo.toml \ --lib --tests --color always {{ arg }} # Clear previous test build artifacts @@ -60,17 +43,17 @@ docs-build open='': # rust docs [group("docs")] docs-rs open='': - cargo doc --no-deps --lib --manifest-path cpp-linter-lib/Cargo.toml {{ open }} + cargo doc --no-deps --lib --manifest-path cpp-linter/Cargo.toml {{ open }} # run cpp-linter native binary [group("bin")] run *args: - cargo run --bin cpp-linter --manifest-path cpp-linter-lib/Cargo.toml -- {{ args }} + cargo run --bin cpp-linter --manifest-path cpp-linter/Cargo.toml -- {{ args }} # build the native binary [group("bin")] build *args='': - cargo build --bin cpp-linter --manifest-path cpp-linter-lib/Cargo.toml {{ args }} + cargo build --bin cpp-linter --manifest-path cpp-linter/Cargo.toml {{ args }} # run clippy and rustfmt lint: diff --git a/cpp-linter-py/.gitignore b/py-binding/.gitignore similarity index 100% rename from cpp-linter-py/.gitignore rename to py-binding/.gitignore diff --git a/cpp-linter-py/Cargo.toml b/py-binding/Cargo.toml similarity index 81% rename from cpp-linter-py/Cargo.toml rename to py-binding/Cargo.toml index b4505c5..5d67efe 100644 --- a/cpp-linter-py/Cargo.toml +++ b/py-binding/Cargo.toml @@ -2,7 +2,7 @@ name = "cpp-linter-py" edition = "2021" readme = "README.md" -repository = "https://github.com/cpp-linter/cpp_linter_rs/tree/main/cpp-linter-py" +repository = "https://github.com/cpp-linter/cpp_linter_rs/tree/main/py-binding" version.workspace = true authors.workspace = true description.workspace = true @@ -17,8 +17,8 @@ crate-type = ["cdylib"] [dependencies] pyo3 = { version = "0.22.3", features = ["extension-module"] } -cpp-linter-lib = { path = "../cpp-linter-lib" } +cpp-linter = { path = "../cpp-linter" } tokio = "1.40.0" [features] -openssl-vendored = ["cpp-linter-lib/openssl-vendored"] +openssl-vendored = ["cpp-linter/openssl-vendored"] diff --git a/cpp-linter-py/README.md b/py-binding/README.md similarity index 100% rename from cpp-linter-py/README.md rename to py-binding/README.md diff --git a/py-binding/cpp_linter/__init__.py b/py-binding/cpp_linter/__init__.py new file mode 100644 index 0000000..f2126fa --- /dev/null +++ b/py-binding/cpp_linter/__init__.py @@ -0,0 +1,14 @@ +# type: ignore +# ruff: noqa: F405 F403 +import sys +from .cpp_linter import * + +__doc__ = cpp_linter.__doc__ +if hasattr(cpp_linter, "__all__"): + __all__ = list(filter(lambda x: x != "main", cpp_linter.__all__)) + + +def main(): + """The main entrypoint for the python frontend. See our rust docs for more info on + the backend (implemented in rust)""" + sys.exit(cpp_linter.main(sys.argv)) diff --git a/cpp-linter-py/pyproject.toml b/py-binding/pyproject.toml similarity index 97% rename from cpp-linter-py/pyproject.toml rename to py-binding/pyproject.toml index bfb865d..25377e4 100644 --- a/cpp-linter-py/pyproject.toml +++ b/py-binding/pyproject.toml @@ -33,7 +33,7 @@ classifiers = [ dynamic = ["version"] [project.scripts] -cpp-linter = "cpp_linter.entry_point:main" +cpp-linter = "cpp_linter:main" [project.urls] source = "https://github.com/cpp-linter/cpp-linter" diff --git a/cpp-linter-py/requirements.txt b/py-binding/requirements.txt similarity index 100% rename from cpp-linter-py/requirements.txt rename to py-binding/requirements.txt diff --git a/py-binding/src/lib.rs b/py-binding/src/lib.rs new file mode 100644 index 0000000..dbe62a0 --- /dev/null +++ b/py-binding/src/lib.rs @@ -0,0 +1,22 @@ +use pyo3::prelude::*; +use tokio::runtime::Builder; + +use ::cpp_linter::run::run_main; + +/// A wrapper for the ``::cpp_linter::run::run_main()``` +#[pyfunction] +fn main(args: Vec) -> PyResult { + Builder::new_multi_thread() + .enable_all() + .build() + .unwrap() + .block_on(async { Ok(run_main(args).await) }) +} + +/// The python binding for the cpp_linter package. It only exposes a ``main()`` function +/// that is used as the entrypoint script. +#[pymodule] +fn cpp_linter(m: &Bound<'_, PyModule>) -> PyResult<()> { + m.add_function(wrap_pyfunction!(main, m)?)?; + Ok(()) +}