Programming exercises: Enable static code analysis for Rust with LocalCI
#10328
Conversation
github-actions
bot
added
tests
server
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
magaupp
force-pushed
the
feature/programming-exercises/rust-sca
branch
from
6b29ddf to
f7a89c0
Compare
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
magaupp
force-pushed
the
feature/programming-exercises/rust-sca
branch
from
f7a89c0 to
304c797
Compare
WalkthroughThe changes introduce static code analysis support for Rust in the Artemis platform. Documentation now reflects that Rust is enabled in the local CI (but not in Jenkins), and the Java backend has been updated to map Rust to the Clippy analysis tool with a new categorizer. A Bash script has been added to run Rust static analysis, build, and test commands, and new test cases along with reference SARIF and JSON reports have been provided to validate the integration. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/test/resources/test-data/static-code-analysis/reports/clippy.sarif (1)
5-468:❓ Verification inconclusive
Verify the completeness of test cases in the results section.
The results section provides a comprehensive set of test cases covering various Clippy lints:
- Error level issues: unsafe code, erasing operations
- Warning level issues: code style, performance, best practices
- Each issue includes detailed location information and helpful suggestions
However, consider adding test cases for:
- Multiple locations for the same issue
- Macro-related issues
- Different severity levels for the same rule
🏁 Script executed:
#!/bin/bash # Check if the test file covers all essential Clippy lint categories ast-grep --pattern 'fn main() { $$$ #[allow(clippy::$_)] $$$ }'Length of output: 71
Action: Manual Verification Required for Additional Test Case Coverage
The SARIF report provides a solid collection of Clippy lint cases (e.g., unsafe code usage, unreachable code, dead code, needless return, etc.), covering various error and warning scenarios. However, note the following suggestions for further enhancing test coverage:
- Multiple Locations for the Same Issue: Validate whether cases with multiple locations have been thoroughly tested.
- Macro-Related Issues: Ensure test cases trigger lints via macros, as these might not be detected by a simple pattern search.
- Different Severity Levels for the Same Rule: Confirm that scenarios exist where the same lint appears with varying severities.
The automated verification script did not produce any output regarding these aspects. Please manually verify the presence of these test cases or consider adding them if they are absent.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/ClippyCategorizer.java (1)
66-837: Extensive Clippy category mapThe large static map provides comprehensive coverage for Clippy lint IDs. As the mapping grows, consider loading it from an external resource (e.g., JSON, YAML) for easier maintenance and updates.
src/main/resources/templates/aeolus/rust/default_static.sh (1)
7-9: Consider adding a comment explaining the jq command.The jq command is performing complex JSON transformations to handle macro expansions. A more detailed comment would help future maintainers understand the transformation logic.
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java (1)
356-356: Consider using a more specific category from ClippyCategorizer.The generic "Style" category might not accurately represent Clippy's actual categorization. Consider using one of the predefined categories from
ClippyCategorizer.CATEGORY_NAMESfor consistency with the rest of the implementation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/main/resources/config/application.ymlis excluded by!**/*.ymlsrc/main/resources/templates/aeolus/rust/default_static.yamlis excluded by!**/*.yamlsrc/main/resources/templates/rust/exercise/Cargo.tomlis excluded by!**/*.tomlsrc/main/resources/templates/rust/solution/Cargo.tomlis excluded by!**/*.toml
📒 Files selected for processing (11)
docs/user/exercises/programming-exercise-features.inc(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/domain/StaticCodeAnalysisTool.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/ParserPolicy.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/ClippyCategorizer.java(1 hunks)src/main/resources/templates/aeolus/rust/default_static.sh(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/StaticCodeAnalysisParserUnitTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java(1 hunks)src/test/resources/test-data/static-code-analysis/expected/clippy.json(1 hunks)src/test/resources/test-data/static-code-analysis/reports/clippy.sarif(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`src/test/java/**/*.java`: test_naming: descriptive; test_si...
src/test/java/**/*.java: test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.javasrc/test/java/de/tum/cit/aet/artemis/programming/StaticCodeAnalysisParserUnitTest.java
`src/main/java/**/*.java`: naming:CamelCase; principles:{sin...
src/main/java/**/*.java: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.javasrc/main/java/de/tum/cit/aet/artemis/programming/domain/StaticCodeAnalysisTool.javasrc/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/ParserPolicy.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/ClippyCategorizer.java
🧠 Learnings (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
Learnt from: magaupp
PR: ls1intum/Artemis#8802
File: src/main/java/de/tum/in/www1/artemis/service/connectors/gitlabci/GitLabCIProgrammingLanguageFeatureService.java:24-24
Timestamp: 2024-11-12T12:51:46.554Z
Learning: Static code analysis for Rust in GitLab CI will be added in a follow-up PR.
src/main/resources/templates/aeolus/rust/default_static.sh (1)
Learnt from: magaupp
PR: ls1intum/Artemis#8802
File: src/main/resources/templates/aeolus/rust/default.sh:14-24
Timestamp: 2024-11-12T12:51:46.554Z
Learning: The `default.sh` script in the `src/main/resources/templates/aeolus/rust/` directory is auto-generated by Aeolus and should not be manually edited. Changes need to be made in the Aeolus generator settings or templates.
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Call Build Workflow / Build .war artifact
- GitHub Check: Call Build Workflow / Build and Push Docker Image
- GitHub Check: client-style
- GitHub Check: server-style
- GitHub Check: server-tests
- GitHub Check: client-tests
- GitHub Check: Analyse
🔇 Additional comments (17)
src/test/resources/test-data/static-code-analysis/reports/clippy.sarif (3)
1-4: LGTM! The SARIF file follows the correct schema.The file correctly references the SARIF 2.1.0 schema and follows the standard structure.
469-555: LGTM! The tool configuration is well-documented.The tool section:
- Correctly identifies Clippy as the analysis tool
- Provides documentation links for each rule
- Includes clear descriptions of rule configurations
556-559: LGTM! Version information is correctly specified.The SARIF version is correctly specified as 2.1.0.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/ClippyCategorizer.java (4)
1-17: Well-structured class definitionThis file cleanly introduces a dedicated categorizer for Rust Clippy lints. The top-level documentation is clear, referencing Rust lint groups and Clippy lint docs. No immediate issues are apparent.
19-21: Enum 'Category' is cohesiveDefining a single enum with both Rust and Clippy categories ensures consistency and reuse. This enumerated approach promotes clarity.
23-32: Minimal, readable categorizeRule implementationThe method handles Clippy vs. Rust rules explicitly, returning a lowercase category name. This is maintainable and clearly expresses intent.
34-62: RUST_CATEGORY_BY_NAME mappingThe mapping is straightforward, using well-labeled lint identifiers. It’s efficient and effectively covers common Rust-level lints. No issues noted.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/ParserPolicy.java (2)
7-7: Import addition is necessaryThe import for
ClippyCategorizeraligns with the new Clippy tool support. This is correct.
32-32: Properly added CLIPPY caseCreating a
SarifParserwithStaticCodeAnalysisTool.CLIPPYandClippyCategorizeris consistent with the other tool cases. Ensure thorough testing of the newly integrated parser.src/main/java/de/tum/cit/aet/artemis/programming/domain/StaticCodeAnalysisTool.java (2)
18-18: Introducing the CLIPPY enum constantNaming follows the established convention, and the file pattern
"clippy.sarif"is appropriately descriptive.
36-36: Registering Clippy for RustAdding
CLIPPYtoTOOLS_OF_PROGRAMMING_LANGUAGElets Rust exercises benefit from static analysis. Good extension of the existing mapping.src/test/java/de/tum/cit/aet/artemis/programming/StaticCodeAnalysisParserUnitTest.java (1)
86-89: LGTM!The test method follows the established pattern in the class and correctly tests the parsing functionality for Clippy reports.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
79-79: LGTM!The change correctly enables static code analysis and testing for Rust in the Local CI environment, aligning with the PR objectives and documentation.
src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java (1)
12-12: LGTM!The changes correctly configure Rust static code analysis by:
- Adding the required import for
ClippyCategorizer- Adding Rust to the map with appropriate categories and tool configuration
Also applies to: 92-92
docs/user/exercises/programming-exercise-features.inc (1)
88-88: LGTM!The documentation correctly reflects that static code analysis for Rust is now supported in Local CI but not in Jenkins, which is consistent with the code changes.
src/main/resources/templates/aeolus/rust/default_static.sh (1)
1-37:✅ Verification successful
Verify if this file should be manually edited.
According to previous learnings, Aeolus auto-generates these scripts. Verify if changes should be made in the Aeolus generator settings or templates instead.
🌐 Web query:
What is Aeolus and how does it handle template generation for build scripts?💡 Result:
Aeolus is a Domain-Specific Language (DSL) designed to streamline the configuration of continuous integration (CI) jobs for programming exercises by generating platform-specific build scripts from a single declarative configuration file[5][8]. Here's how it handles template generation:
Core Functionality
- Declarative Configuration: Users define CI workflows in a YAML-based Windfile that specifies:
- Docker images and tags
- Git repositories/branches
- Custom actions (script blocks or prebuilt templates)
- Environment variables[5][8]
Template Generation Process
Multi-Platform Support: Converts Windfiles into executable configurations for:
- Jenkins: Full pipeline definitions with stages/steps
- Bamboo: Build plans with tasks/dependencies
- Bash: Portable shell scripts with error handling[5][8]
Action Templating:
actions: - name: test-suite use: https://github.com/example/ci-templates.git parameters: TEST_TYPE: "unit" - name: security-scan script: ./run_scanner.shThis generates platform-specific implementations while preserving parameterization[5][8].
Lifecycle Management:
- Automates environment setup/teardown
- Handles artifact cleanup
- Manages conditional execution paths[5]
Key Features
Capability Implementation Platform Abstraction Single config → multiple CI systems Template Reuse Shared action repositories via Git Environment Control Docker containerization per stage Migration Support Translates existing Bamboo/Jenkins configs to Windfiles[8] Aeolus reduces CI maintenance overhead by 83% for multi-platform environments according to internal benchmarks[8], while ensuring consistency across different execution environments through its template-driven approach.
Citations:
- 1: https://en.wikipedia.org/wiki/Aeolus
- 2: https://aeolusproject.github.io/configure.html
- 3: https://www.ssec.wisc.edu/grafiir/AEOLUS_WIND/L2B_Documentation/AE-MA-ECMWF-L2BP-001_SUM_20170906_v1.80.pdf
- 4: https://study.com/academy/lesson/the-greek-god-aeolus-mythology-lesson-quiz.html
- 5: https://ls1intum.github.io/Aeolus/
- 6: https://docs.aveva.com/bundle/sp-omi/page/643522.html
- 7: https://en.wiktionary.org/wiki/Aeolus
- 8: https://github.com/ls1intum/Aeolus
- 9: https://cdn.knmi.nl/system/data_center_publications/files/000/067/433/original/aemaecmwfl2bp001_20080229_sum_iss1.33.pdf?1495620766
- 10: https://www.theoi.com/Titan/Aiolos.html
Important: This auto-generated script should not be manually edited.
Based on the Aeolus documentation and our web query, this file is generated automatically from configuration settings (such as your Windfile) that define CI workflows. Therefore, manual modifications to this script are discouraged. Any necessary changes should be made in the Aeolus generator settings or its associated templates to ensure that they persist through regeneration.
- File Affected:
src/main/resources/templates/aeolus/rust/default_static.sh- Action Required: Update the Aeolus configuration (e.g., Windfile) or generator templates instead of manually editing this script.
src/test/resources/test-data/static-code-analysis/expected/clippy.json (1)
1-137: LGTM! Comprehensive test data covering various Clippy scenarios.The test data effectively covers:
- Different severity levels (errors and warnings)
- Various Clippy categories (unsafe_code, suspicious, unused, etc.)
- Different types of issues (unsafe blocks, unreachable code, style violations, etc.)
- Detailed issue information (file paths, line numbers, messages)
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
# Conflicts: # src/main/java/de/tum/cit/aet/artemis/programming/domain/StaticCodeAnalysisTool.java # src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/ParserPolicy.java # src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java
magaupp
dismissed stale reviews from SimonEntholzer, iyannsch, simonbohnen, az108, and coderabbitai[bot]
via
40b840d
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/user/exercises/programming-exercise-features.inc (1)
121-122: Consider updating the note about C language.Since Rust now joins C in having LocalCI-only static analysis support, consider updating this note to be more generic.
- Static Code Analysis for ``C`` exercises is only supported for ``LocalCI`` at the moment. + Static Code Analysis for some languages (e.g., ``C``, ``Rust``) is only supported for ``LocalCI`` at the moment.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/main/resources/config/application.ymlis excluded by!**/*.yml
📒 Files selected for processing (7)
docs/user/exercises/programming-exercise-features.inc(1 hunks)src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/domain/StaticCodeAnalysisTool.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/ParserPolicy.java(2 hunks)src/test/java/de/tum/cit/aet/artemis/programming/StaticCodeAnalysisParserUnitTest.java(1 hunks)src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java
- src/main/java/de/tum/cit/aet/artemis/programming/domain/StaticCodeAnalysisTool.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/ParserPolicy.java
- src/test/java/de/tum/cit/aet/artemis/programming/StaticCodeAnalysisParserUnitTest.java
- src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java
- src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java
🔇 Additional comments (1)
docs/user/exercises/programming-exercise-features.inc (1)
88-88: LGTM! The Rust feature update accurately reflects the implementation.The change correctly indicates that static code analysis for Rust is now supported in LocalCI but not in Jenkins (
L: yes; J: no), which aligns with:
- The PR objectives of configuring Clippy for LocalCI
- The implementation details in the backend changes
- The pattern used for other languages with similar support levels (e.g., C, Ruby, Dart)
Checklist
General
Server
Changes affecting Programming Exercises
Description
This PR configures Clippy with clippy-sarif to enable static code analysis for Rust.
Cargo's JSON output is preprocessed with
jqbecauseclippy-sarifdoesn't handle multiple locations per issue and macro invocations correctly.The ClippyCategorizer maps some Rust compiler lints and all Clippy lints to their lint groups.
Steps for Testing
Prerequisites:
fn foo(bar: &Box<i32>) {}into a file of the solution repositoryclippy::borrowed_boxcode issue is in the Solution feedbackTestserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Test Coverage
Server
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Documentation
Tests