From 48a38dc034d9356bb9c6bf297df31b7f51d3a62e Mon Sep 17 00:00:00 2001 From: Lifei Zhou Date: Sun, 3 Aug 2025 20:03:08 +1000 Subject: [PATCH] Chore: apply more clippy rules to prevent from code complexity (#3813) --- .github/workflows/ci.yml | 2 +- CONTRIBUTING.md | 2 +- clippy-baselines/cognitive_complexity.txt | 67 +++++++++ clippy-baselines/too_many_lines.txt | 60 ++++++++ crates/mcp-client/src/client.rs | 4 +- rust-toolchain.toml | 2 +- scripts/clippy-baseline.sh | 160 ++++++++++++++++++++++ scripts/clippy-lint.sh | 24 ++++ 8 files changed, 316 insertions(+), 5 deletions(-) create mode 100644 clippy-baselines/cognitive_complexity.txt create mode 100644 clippy-baselines/too_many_lines.txt create mode 100755 scripts/clippy-baseline.sh create mode 100755 scripts/clippy-lint.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index dc7f03815a..612b9c66c0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -89,7 +89,7 @@ jobs: run: | source ./bin/activate-hermit export CARGO_INCREMENTAL=0 - cargo clippy --jobs 2 -- -D warnings + ./scripts/clippy-lint.sh - name: Install Node.js Dependencies for OpenAPI Check run: source ../../bin/activate-hermit && npm ci diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 54959eb514..817f777db6 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -61,7 +61,7 @@ As you make changes to the rust code, you can try it out on the CLI, or also run cargo check # do your changes compile cargo test # do the tests pass with your changes cargo fmt # format your code -cargo clippy # run the linter +./scripts/clippy-lint # run the linter ``` ### Node diff --git a/clippy-baselines/cognitive_complexity.txt b/clippy-baselines/cognitive_complexity.txt new file mode 100644 index 0000000000..68fe284df0 --- /dev/null +++ b/clippy-baselines/cognitive_complexity.txt @@ -0,0 +1,67 @@ +crates/goose-bench/src/runners/eval_runner.rs::run +crates/goose-bench/src/runners/metric_aggregator.rs::generate_csv_from_benchmark_dir +crates/goose-bench/src/utilities.rs::await_process_exits +crates/goose-cli/src/cli.rs::cli +crates/goose-cli/src/commands/web.rs::handle_socket +crates/goose-cli/src/commands/web.rs::process_message_streaming +crates/goose-cli/src/session/builder.rs::build_session +crates/goose-cli/src/session/mod.rs::interactive +crates/goose-cli/src/session/mod.rs::process_agent_response +crates/goose-mcp/src/computercontroller/docx_tool.rs::docx_tool +crates/goose-mcp/src/computercontroller/pdf_tool.rs::pdf_tool +crates/goose-mcp/src/google_drive/mod.rs::docs_tool +crates/goose-mcp/src/google_drive/mod.rs::google_auth +crates/goose-mcp/src/google_drive/oauth_pkce.rs::perform_oauth_flow +crates/goose-mcp/src/google_drive/storage.rs::write_to_file +crates/goose-server/src/commands/agent.rs::run +crates/goose-server/src/openapi.rs::convert_typed_schema +crates/goose-server/src/openapi.rs::convert_typed_schema +crates/goose-server/src/routes/audio.rs::transcribe_elevenlabs_handler +crates/goose-server/src/routes/audio.rs::transcribe_elevenlabs_handler +crates/goose-server/src/routes/audio.rs::transcribe_handler +crates/goose-server/src/routes/audio.rs::transcribe_handler +crates/goose-server/src/routes/config_management.rs::get_pricing +crates/goose-server/src/routes/config_management.rs::get_pricing +crates/goose-server/src/routes/extension.rs::add_extension +crates/goose-server/src/routes/extension.rs::add_extension +crates/goose-server/src/routes/session.rs::get_session_insights +crates/goose-server/src/routes/session.rs::get_session_insights +crates/goose-server/src/routes/setup.rs::start_openrouter_setup +crates/goose-server/src/routes/setup.rs::start_openrouter_setup +crates/goose/src/agents/extension_manager.rs::merge_environments +crates/goose/src/agents/retry.rs::execute_on_failure_command +crates/goose/src/agents/retry.rs::handle_retry_logic +crates/goose/src/config/base.rs::try_restore_from_backup +crates/goose/src/context_mgmt/truncate.rs::truncate_messages +crates/goose/src/providers/anthropic.rs::post +crates/goose/src/providers/azure.rs::post +crates/goose/src/providers/claude_code.rs::find_claude_executable +crates/goose/src/providers/databricks.rs::post_with_retry +crates/goose/src/providers/gcpvertexai.rs::post_with_location +crates/goose/src/providers/gemini_cli.rs::find_gemini_executable +crates/goose/src/providers/githubcopilot.rs::poll_for_access_token +crates/goose/src/providers/lead_worker.rs::detect_task_failures +crates/goose/src/providers/lead_worker.rs::handle_completion_result +crates/goose/src/providers/oauth.rs::get_oauth_token_async +crates/goose/src/providers/pricing.rs::load_from_disk +crates/goose/src/providers/snowflake.rs::post +crates/goose/src/providers/venice.rs::post +crates/goose/src/providers/xai.rs::post +crates/goose/src/scheduler_factory.rs::create +crates/goose/src/scheduler_factory.rs::from_config +crates/goose/src/scheduler.rs::add_scheduled_job +crates/goose/src/scheduler.rs::kill_running_job +crates/goose/src/scheduler.rs::load_jobs_from_storage +crates/goose/src/scheduler.rs::run_scheduled_job_internal +crates/goose/src/session/storage.rs::parse_message_with_truncation +crates/goose/src/session/storage.rs::read_messages_with_truncation +crates/goose/src/temporal_scheduler.rs::discover_http_port +crates/goose/src/temporal_scheduler.rs::find_go_service_binary +crates/goose/src/temporal_scheduler.rs::new +crates/goose/src/temporal_scheduler.rs::start_go_service +crates/goose/src/temporal_scheduler.rs::stop_services +crates/goose/src/temporal_scheduler.rs::update_job_status_from_sessions +crates/goose/src/temporal_scheduler.rs::wait_for_service_ready +crates/mcp-client/src/oauth.rs::execute +crates/mcp-client/src/oauth.rs::get_oauth_endpoints +crates/mcp-server/src/lib.rs::run diff --git a/clippy-baselines/too_many_lines.txt b/clippy-baselines/too_many_lines.txt new file mode 100644 index 0000000000..13142c9221 --- /dev/null +++ b/clippy-baselines/too_many_lines.txt @@ -0,0 +1,60 @@ +crates/goose-bench/src/eval_suites/core/developer/simple_repo_clone_test.rs::run +crates/goose-cli/src/cli.rs::cli +crates/goose-cli/src/commands/configure.rs::configure_extensions_dialog +crates/goose-cli/src/commands/configure.rs::configure_provider_dialog +crates/goose-cli/src/commands/configure.rs::configure_tool_permissions_dialog +crates/goose-cli/src/commands/configure.rs::handle_configure +crates/goose-cli/src/commands/project.rs::handle_project_default +crates/goose-cli/src/commands/project.rs::handle_projects_interactive +crates/goose-cli/src/commands/web.rs::handle_socket +crates/goose-cli/src/commands/web.rs::process_message_streaming +crates/goose-cli/src/session/builder.rs::build_session +crates/goose-cli/src/session/export.rs::tool_response_to_markdown +crates/goose-cli/src/session/mod.rs::handle_interrupted_messages +crates/goose-cli/src/session/mod.rs::interactive +crates/goose-cli/src/session/mod.rs::process_agent_response +crates/goose-mcp/src/computercontroller/docx_tool.rs::docx_tool +crates/goose-mcp/src/computercontroller/mod.rs::new +crates/goose-mcp/src/computercontroller/mod.rs::quick_script +crates/goose-mcp/src/computercontroller/mod.rs::xlsx_tool +crates/goose-mcp/src/computercontroller/pdf_tool.rs::pdf_tool +crates/goose-mcp/src/developer/mod.rs::bash +crates/goose-mcp/src/developer/mod.rs::new +crates/goose-mcp/src/google_drive/google_labels.rs::doit +crates/goose-mcp/src/google_drive/mod.rs::create_file +crates/goose-mcp/src/google_drive/mod.rs::docs_tool +crates/goose-mcp/src/google_drive/mod.rs::new +crates/goose-mcp/src/google_drive/mod.rs::search_files +crates/goose-mcp/src/google_drive/mod.rs::sharing +crates/goose-mcp/src/google_drive/mod.rs::sheets_tool +crates/goose-mcp/src/google_drive/mod.rs::update_label +crates/goose-mcp/src/memory/mod.rs::new +crates/goose-server/src/openapi.rs::convert_typed_schema +crates/goose-server/src/openapi.rs::convert_typed_schema +crates/goose-server/src/routes/audio.rs::transcribe_elevenlabs_handler +crates/goose-server/src/routes/audio.rs::transcribe_elevenlabs_handler +crates/goose-server/src/routes/extension.rs::add_extension +crates/goose-server/src/routes/extension.rs::add_extension +crates/goose-server/src/routes/extension.rs::is_command_allowed_with_allowlist +crates/goose-server/src/routes/extension.rs::is_command_allowed_with_allowlist +crates/goose-server/src/routes/reply.rs::reply_handler +crates/goose-server/src/routes/reply.rs::reply_handler +crates/goose/src/agents/agent.rs::create_recipe +crates/goose/src/agents/agent.rs::reply_internal +crates/goose/src/agents/extension_manager.rs::add_extension +crates/goose/src/providers/azure.rs::post +crates/goose/src/providers/databricks.rs::post_with_retry +crates/goose/src/providers/formats/anthropic.rs::format_messages +crates/goose/src/providers/formats/anthropic.rs::response_to_streaming_message +crates/goose/src/providers/formats/databricks.rs::format_messages +crates/goose/src/providers/formats/google.rs::format_messages +crates/goose/src/providers/formats/openai.rs::format_messages +crates/goose/src/providers/formats/openai.rs::response_to_streaming_message +crates/goose/src/providers/gcpvertexai.rs::post_with_location +crates/goose/src/providers/snowflake.rs::post +crates/goose/src/scheduler.rs::add_scheduled_job +crates/goose/src/scheduler.rs::load_jobs_from_storage +crates/goose/src/scheduler.rs::run_scheduled_job_internal +crates/goose/src/scheduler.rs::update_schedule +crates/goose/src/session/storage.rs::read_messages_with_truncation +crates/mcp-server/src/lib.rs::run diff --git a/crates/mcp-client/src/client.rs b/crates/mcp-client/src/client.rs index e125385516..78ff767ff5 100644 --- a/crates/mcp-client/src/client.rs +++ b/crates/mcp-client/src/client.rs @@ -62,7 +62,7 @@ impl ClientHandler for GooseClient { &self, params: rmcp::model::ProgressNotificationParam, context: rmcp::service::NotificationContext, - ) -> () { + ) { self.notification_handlers .lock() .await @@ -82,7 +82,7 @@ impl ClientHandler for GooseClient { &self, params: rmcp::model::LoggingMessageNotificationParam, context: rmcp::service::NotificationContext, - ) -> () { + ) { self.notification_handlers .lock() .await diff --git a/rust-toolchain.toml b/rust-toolchain.toml index 02cb8fcb53..ab40f4f445 100644 --- a/rust-toolchain.toml +++ b/rust-toolchain.toml @@ -1,3 +1,3 @@ [toolchain] -channel = "stable" +channel = "1.88.0" profile = "default" diff --git a/scripts/clippy-baseline.sh b/scripts/clippy-baseline.sh new file mode 100755 index 0000000000..e66c7176c4 --- /dev/null +++ b/scripts/clippy-baseline.sh @@ -0,0 +1,160 @@ +#!/bin/bash + +# Baseline clippy rules - only fail on NEW violations +# +# Format: "rule_name|violation_parser" +# +# Violation parsers (run clippy on your rule to see which fits): +# function_name - When spans show: "fn my_function(..." +# type_name - When spans show: "struct MyStruct" or "enum MyEnum" +# file_only - When spans show file-level issues +# +# Note: If your rule doesn't fit these parsers, you may need to add a new parser +# to the parse_violation() function below +# +# To add new rules: +# 1. Add rule below: "clippy::your_rule|violation_parser" +# 2. Generate baseline: ./scripts/clippy-baseline.sh generate clippy::your_rule + +BASELINE_RULES=( + "clippy::cognitive_complexity|function_name" + "clippy::too_many_lines|function_name" +) + +parse_violation() { + local rule_code="$1" + local violation_parser="$2" + + case "$violation_parser" in + "function_name") + jq -r 'select(.message.code.code == "'"$rule_code"'") | + "\(.message.spans[0].file_name)::\(.message.spans[0].text[0].text | split("fn ")[1] | split("(")[0])"' + ;; + "type_name") + jq -r 'select(.message.code.code == "'"$rule_code"'") | + "\(.message.spans[0].file_name)::\(.message.spans[0].text[0].text | split(" ")[1] | split(" ")[0])"' + ;; + "file_only") + jq -r 'select(.message.code.code == "'"$rule_code"'") | + "\(.message.spans[0].file_name)"' + ;; + *) + echo "Unknown violation parser: $violation_parser" >&2 + exit 1 + ;; + esac +} + +get_baseline_file() { + local rule_name="$1" + local safe_name=$(echo "$rule_name" | sed 's/clippy:://' | sed 's/:/-/g') + echo "clippy-baselines/${safe_name}.txt" +} + + +generate_baseline() { + local rule_name="$1" + + [[ -z "$rule_name" ]] && { echo "Missing rule name"; return 1; } + + local violation_parser="" + for rule in "${BASELINE_RULES[@]}"; do + [[ "${rule%|*}" == "$rule_name" ]] && { violation_parser="${rule#*|}"; break; } + done + + [[ -z "$violation_parser" ]] && { echo "Unknown rule: $rule_name"; return 1; } + + local baseline_file=$(get_baseline_file "$rule_name") + + cargo clippy --jobs 2 --message-format=json -- -W "$rule_name" | \ + parse_violation "$rule_name" "$violation_parser" | \ + sort > "$baseline_file" + + echo "✅ Generated baseline for $rule_name ($(wc -l < "$baseline_file") violations)" +} + + +# Check a single rule from pre-generated JSON (optimized version) +check_rule_from_json() { + local temp_json="$1" + local rule_name="$2" + local violation_parser="$3" + local baseline_file="$4" + + echo " → Checking $rule_name" + + if [[ ! -f "$baseline_file" ]]; then + echo " ❌ $rule_name: baseline file not found" + return 1 + fi + + local temp_parsed=$(mktemp) + cat "$temp_json" | parse_violation "$rule_name" "$violation_parser" | sort > "$temp_parsed" + + local new_violations_file=$(mktemp) + diff <(sort "$baseline_file") <(sort "$temp_parsed") | grep "^>" | cut -c3- > "$new_violations_file" + + if [[ -s "$new_violations_file" ]]; then + echo " ❌ $rule_name: NEW violations found:" + + while IFS= read -r violation; do + # Extract all violations for this rule and find the matching one + cat "$temp_json" | jq -c 'select(.message.code.code == "'"$rule_name"'")' 2>/dev/null | while read -r json_line; do + parsed_id=$(echo "$json_line" | parse_violation "$rule_name" "$violation_parser") + if [[ "$parsed_id" == "$violation" ]]; then + echo "$json_line" | jq -r '.message.rendered' | sed 's/^/ /' + fi + done + done < "$new_violations_file" + + rm "$temp_parsed" "$new_violations_file" + return 1 + fi + + rm "$new_violations_file" + + echo " ✅ $rule_name: ok" + rm "$temp_parsed" + return 0 +} + +check_all_baseline_rules() { + echo "🔍 Checking baseline clippy rules..." + + local clippy_flags="" + for rule in "${BASELINE_RULES[@]}"; do + local rule_name="${rule%|*}" + clippy_flags="$clippy_flags -W $rule_name" + done + + local temp_json=$(mktemp) + cargo clippy --jobs 2 --message-format=json -- $clippy_flags > "$temp_json" + + local failed_rules=() + + # Check each rule against its baseline + for rule in "${BASELINE_RULES[@]}"; do + local rule_name="${rule%|*}" + local violation_parser="${rule#*|}" + local baseline_file=$(get_baseline_file "$rule_name") + + if ! check_rule_from_json "$temp_json" "$rule_name" "$violation_parser" "$baseline_file"; then + failed_rules+=("$rule_name") + fi + done + + rm "$temp_json" + + if [[ ${#failed_rules[@]} -gt 0 ]]; then + echo "" + echo "❌ Failed baseline checks for: ${failed_rules[*]}" + exit 1 + else + echo "" + echo "✅ All baseline clippy checks passed!" + fi +} + +if [[ "$1" == "generate" ]]; then + generate_baseline "$2" +fi \ No newline at end of file diff --git a/scripts/clippy-lint.sh b/scripts/clippy-lint.sh new file mode 100755 index 0000000000..d44b669df4 --- /dev/null +++ b/scripts/clippy-lint.sh @@ -0,0 +1,24 @@ +#!/bin/bash + +# Combined lint script +# Runs standard clippy (strict) + baseline clippy rules + +set -e + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +# Source the baseline functions +source "$SCRIPT_DIR/clippy-baseline.sh" + +echo "🔍 Running all clippy checks..." + +# Run standard clippy with strict warnings +echo " → Standard clippy rules (strict)" +cargo clippy --jobs 2 -- -D warnings + +# Run baseline rules check +echo "" +check_all_baseline_rules + +echo "" +echo "✅ All lint checks passed!" \ No newline at end of file