Programming exercises: Enable static code analysis for JavaScript with LocalCI
#10372
Conversation
github-actions
bot
added
tests
server
magaupp
changed the base branch from
develop
to
feature/programming-exercises/sca-tool-eslint
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
WalkthroughThis pull request refines the configuration and documentation for JavaScript static code analysis within the Artemis framework. It updates the programming exercise feature tables, adds ESLint support via new config entries and enum constants, adjusts language feature flags, and introduces a new parser categorizer. Additionally, new scripts, configuration files, test cases, and expected report files are provided to support ESLint-based analysis on both local and CI environments. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User/CI
participant S as default_static.sh
participant L as LocalCIProgrammingLanguageFeatureService
participant C as StaticCodeAnalysisConfigurer
participant P as ParserPolicy
participant SC as SingleCategoryCategorizer
participant E as ESLint Tool
U->>S: Trigger build/analysis
S->>S: Install dependencies
S->>S: Run static_code_analysis (lint/test)
S->>L: Request JavaScript feature configuration
L->>C: Retrieve ESLint config mapping
C->>P: Parse ESLint SARIF report
P->>SC: Categorize rules using GENERIC_LINT_CATEGORY
SC-->>P: Return categorized results
P-->>S: Deliver parsed analysis outcome
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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: 5
🧹 Nitpick comments (4)
src/test/resources/test-data/static-code-analysis/reports/eslint.sarif (1)
10-27: Consider expanding the ESLint ruleset.Currently, only two basic rules are enabled:
- no-debugger
- no-empty
Consider adding more essential rules for JavaScript, such as:
- no-unused-vars
- no-undef
- eqeqeq
- prefer-const
src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java (1)
149-149: Address TODO comment for Swift categories.While unrelated to this PR, there's an outstanding TODO comment for Swift categories that should be addressed.
Would you like me to create an issue to track the implementation of correct category rules for Swift?
src/test/resources/test-data/static-code-analysis/expected/eslint.json (1)
5-5: Consider using relative paths in test fixtures.The file paths in the test fixture use absolute paths (
/var/tmp/testing-dir/assignment/src/client.js). Consider using relative paths to make the tests more portable across different environments.Apply this diff to use relative paths:
- "filePath": "/var/tmp/testing-dir/assignment/src/client.js", + "filePath": "src/client.js",Also applies to: 16-16
src/main/resources/templates/javascript/staticCodeAnalysis/test/package.json (1)
4-9: Verify ESLint command usage and environment variable substitution.The lint scripts reference the variable
${studentParentWorkingDirectoryName}. Please ensure this variable is defined in your environment or provided via npm configuration so that the commands execute without errors.
📜 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/javascript/default_static.yamlis excluded by!**/*.yamlsrc/main/resources/templates/javascript/staticCodeAnalysis/test/package-lock.jsonis excluded by!**/package-lock.jsonsrc/main/resources/templates/javascript/test/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (13)
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/SingleCategoryCategorizer.java(1 hunks)src/main/resources/templates/aeolus/javascript/default_static.sh(1 hunks)src/main/resources/templates/javascript/staticCodeAnalysis/test/eslint.config.mjs(1 hunks)src/main/resources/templates/javascript/staticCodeAnalysis/test/package.json(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/eslint.json(1 hunks)src/test/resources/test-data/static-code-analysis/reports/eslint.sarif(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`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/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/SingleCategoryCategorizer.javasrc/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java
`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/StaticCodeAnalysisParserUnitTest.javasrc/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java
🧠 Learnings (2)
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/ParserPolicy.java (1)
Learnt from: magaupp
PR: ls1intum/Artemis#10204
File: src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/RubocopCategorizer.java:13-15
Timestamp: 2025-01-28T17:50:58.545Z
Learning: In the Artemis codebase, null checks for ReportingDescriptor and rule.id() are handled during SARIF parsing before reaching the categorizer classes.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/SingleCategoryCategorizer.java (1)
Learnt from: magaupp
PR: ls1intum/Artemis#10204
File: src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/RubocopCategorizer.java:13-15
Timestamp: 2025-01-28T17:50:58.545Z
Learning: In the Artemis codebase, null checks for ReportingDescriptor and rule.id() are handled during SARIF parsing before reaching the categorizer classes.
🔇 Additional comments (14)
src/main/resources/templates/javascript/staticCodeAnalysis/test/eslint.config.mjs (1)
1-9:❓ Verification inconclusive
LGTM! Verify package versions for security.
The ESLint configuration follows best practices using the new flat config format. The setup includes Node.js globals and recommended rules, providing a solid foundation for JavaScript static analysis.
Let's verify the versions of the required packages:
🏁 Script executed:
#!/bin/bash # Check latest versions of ESLint and its dependencies npm view eslint version npm view globals version npm view @eslint/js version # Check for any security advisories npm audit --package-lock-onlyLength of output: 484
LGTM! The ESLint configuration is solid and uses the new flat config format correctly.
- The dependency versions are confirmed as follows:
- ESLint: 9.20.1
- globals: 16.0.0
- @eslint/js: 9.20.0
- Note that the
npm auditstep failed due to the absence of a package-lock file. For complete security verification, please create a package-lock file (e.g., usingnpm i --package-lock-only) or manually run an audit.src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/SingleCategoryCategorizer.java (1)
1-20: LGTM! Clean and focused implementation.The class follows SOLID principles with a single responsibility, is immutable, and thread-safe. The implementation is simple yet effective for categorizing all rules under a single category.
src/main/java/de/tum/cit/aet/artemis/programming/domain/StaticCodeAnalysisTool.java (1)
20-20: LGTM! Consistent integration of ESLint support.The changes correctly integrate ESLint support by following the established patterns in the codebase:
- The enum constant uses the correct SARIF filename
- The tool is properly mapped to JavaScript in the TOOLS_OF_PROGRAMMING_LANGUAGE map
Also applies to: 37-37
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/ParserPolicy.java (1)
20-24: LGTM! Well-integrated ESLint support.The changes properly integrate ESLint support:
- The GENERIC_LINT_CATEGORY constant is well-documented
- The ESLINT case follows the established pattern for SARIF parsers
- The implementation is consistent with other static analysis tools
Also applies to: 41-41
src/test/resources/test-data/static-code-analysis/reports/eslint.sarif (1)
1-110: LGTM! The SARIF report structure is well-formed and complete.The report follows the SARIF 2.1.0 schema and includes all necessary components:
- Tool metadata with version information
- Rule definitions with help URIs
- Precise issue locations with line/column information
- Clear error messages
src/test/java/de/tum/cit/aet/artemis/programming/StaticCodeAnalysisParserUnitTest.java (1)
66-69: LGTM! Test method follows established patterns.The test method:
- Has a clear, descriptive name
- Follows the same pattern as other parser tests
- Tests the essential functionality
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
72-72: LGTM! JavaScript feature configuration updated correctly.The change enables static code analysis for JavaScript while maintaining other feature flags, aligning with the PR's objective to implement ESLint support.
src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java (1)
95-95: LGTM! JavaScript static analysis configuration added correctly.The configuration:
- Uses appropriate generic lint category
- Associates with ESLint tool
- Follows the pattern used by other languages
docs/user/exercises/programming-exercise-features.inc (1)
90-90: LGTM!The documentation update correctly reflects the addition of static code analysis support for JavaScript in LocalCI.
src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java (1)
358-358: LGTM!The addition of the ESLint case with the "Lint" category is consistent with ESLint's categorization scheme and aligns with the test data in
eslint.json.src/main/resources/templates/javascript/staticCodeAnalysis/test/package.json (4)
1-3: Approved basic structure and metadata.The project name, privacy flag, and overall JSON structure are correctly set up for the new testing template.
10-12: Confirm workspace configuration for dynamic directory.The "workspaces" field uses
${studentParentWorkingDirectoryName}similarly to the lint scripts. Double-check that it is correctly substituted during execution to avoid misconfiguration in multi-package setups.
13-24: Check dependency versions and integration.The devDependencies are well specified with pinned versions (using caret notation) for Babel, ESLint, Jest, and related packages. Make sure these versions are compatible with one another and that they meet the project’s requirements, especially regarding ESLint integration.
25-29: Review jest-junit configuration details.The jest-junit settings (templates and ancestor separator) appear to be correctly customized. Verify that these configurations produce the expected report format in your CI environment.
helios-aet
bot
temporarily deployed
to
artemis-test4.artemis.cit.tum.de
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
Note
Large diff due to
package-lock.jsonupdateChecklist
General
Server
Changes affecting Programming Exercises
Motivation and Context
This PR enables static code analysis for JavaScript using ESLint.
Rules can be configured in the
eslint.config.mjsfile of the Test repository.Description
ESLint does not assign any category to the lints, so each code issue will have the category
Lint.The docker image is updated to include the new packages in the package cache.
Steps for Testing
Prerequisites:
Edit in Editorbutton on the top{}) to a functionno-emptycode issue is in the 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
New Features
Documentation
Tests