Programming exercises: Enable static code analysis for TypeScript with LocalCI
#10373
Conversation
github-actions
bot
added
server
helios-aet
bot
temporarily deployed
to
artemis-test3.artemis.cit.tum.de
magaupp
changed the title
Programming exercises Enable static code analysis for TypeScript with LocalCIProgramming exercises: Enable static code analysis for TypeScript with LocalCI
helios-aet
bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
WalkthroughThis pull request updates the support for TypeScript within the static code analysis and continuous integration systems. The documentation now reflects that TypeScript supports static code analysis under Local CI. In the core configuration, a new mapping for TypeScript is added using ESLint, and a corresponding enumeration value for ESLint is introduced. The parser logic is also updated with a new categorizer and constant to handle ESLint issues. Additionally, new template files, ESLint configurations, and test cases have been added to fully integrate and validate the ESLint support. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CI_System as LocalCI Feature Service
participant Configurer as StaticCodeAnalysisConfigurer
participant Parser as ParserPolicy
participant Categorizer as SingleCategoryCategorizer
Developer->>CI_System: Commit TypeScript code and trigger build
CI_System->>Configurer: Retrieve static analysis configuration for TypeScript
Configurer-->>CI_System: Return configuration mapping (ESLint for TypeScript)
CI_System->>Parser: Parse ESLint SARIF report
Parser->>Categorizer: Categorize rules using generic lint category
Categorizer-->>Parser: Return categorized issue
Parser-->>CI_System: Report static analysis results
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: 0
🧹 Nitpick comments (5)
src/test/resources/test-data/static-code-analysis/reports/eslint.sarif (1)
31-57: Consider using relative paths in test data.The use of absolute paths with
/var/tmp/testing-dir/in test data might cause portability issues across different environments.Consider using relative paths or environment variables to make the test data more portable:
- "uri": "file:///var/tmp/testing-dir/assignment/src/bubblesort.js" + "uri": "file:///${TEST_DIR}/assignment/src/bubblesort.js"src/main/resources/templates/typescript/staticCodeAnalysis/test/eslint.config.mjs (1)
8-9: Consider restricting Node.js globals.The current configuration allows all Node.js globals, which might be too permissive for student exercises. Consider explicitly listing only the required globals.
- {languageOptions: { globals: globals.node }}, + {languageOptions: { globals: { + // List only required globals + console: false, + process: false + }}},src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/SingleCategoryCategorizer.java (1)
5-7: Enhance class documentation.Consider adding more detailed documentation explaining:
- The purpose of using a single category
- Example use cases
- Whether null categories are allowed
/** * Categorizes all rules into the same category. + * + * This categorizer is useful when a static analysis tool doesn't provide meaningful + * categorization or when all issues should be grouped under a single category. + * + * @param category The category to assign to all rules. Must not be null. */src/main/resources/templates/aeolus/typescript/default_static.sh (1)
30-37: Consider optimizing directory handling.The script repeatedly changes back to the initial directory between each command. Consider wrapping the commands in a subshell to avoid multiple directory changes:
- cd "${AEOLUS_INITIAL_DIRECTORY}" - bash -c "source ${_script_name} aeolus_sourcing; install_dependencies" - cd "${AEOLUS_INITIAL_DIRECTORY}" - bash -c "source ${_script_name} aeolus_sourcing; static_code_analysis" - cd "${AEOLUS_INITIAL_DIRECTORY}" - bash -c "source ${_script_name} aeolus_sourcing; build" - cd "${AEOLUS_INITIAL_DIRECTORY}" - bash -c "source ${_script_name} aeolus_sourcing; test" + ( + cd "${AEOLUS_INITIAL_DIRECTORY}" + bash -c "source ${_script_name} aeolus_sourcing; install_dependencies" + bash -c "source ${_script_name} aeolus_sourcing; static_code_analysis" + bash -c "source ${_script_name} aeolus_sourcing; build" + bash -c "source ${_script_name} aeolus_sourcing; test" + )src/main/resources/templates/typescript/staticCodeAnalysis/test/package.json (1)
15-25: Consider pinning dependency versions.Using caret ranges (
^) for dependencies may lead to unexpected behavior if new versions introduce breaking changes. Consider pinning exact versions for better reproducibility in CI environment."devDependencies": { - "@eslint/js": "^9.20.0", - "@microsoft/eslint-formatter-sarif": "^3.1.0", - "@tsconfig/node20": "^20.1.4", - "@types/jest": "^29.5.12", - "eslint": "^9.20.1", - "globals": "^15.15.0", - "jest": "^29.7.0", - "jest-junit": "^16.0.0", - "ts-jest": "^29.2.5", - "typescript": "^5.6.2", - "typescript-eslint": "^8.24.1" + "@eslint/js": "9.20.0", + "@microsoft/eslint-formatter-sarif": "3.1.0", + "@tsconfig/node20": "20.1.4", + "@types/jest": "29.5.12", + "eslint": "9.20.1", + "globals": "15.15.0", + "jest": "29.7.0", + "jest-junit": "16.0.0", + "ts-jest": "29.2.5", + "typescript": "5.6.2", + "typescript-eslint": "8.24.1"
📜 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/typescript/default_static.yamlis excluded by!**/*.yamlsrc/main/resources/templates/typescript/staticCodeAnalysis/test/package-lock.jsonis excluded by!**/package-lock.jsonsrc/main/resources/templates/typescript/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/typescript/default_static.sh(1 hunks)src/main/resources/templates/typescript/staticCodeAnalysis/test/eslint.config.mjs(1 hunks)src/main/resources/templates/typescript/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/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
`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/service/localci/scaparser/strategy/sarif/SingleCategoryCategorizer.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.java
🧠 Learnings (3)
src/main/resources/templates/typescript/staticCodeAnalysis/test/package.json (4)
Learnt from: magaupp
PR: ls1intum/Artemis#9440
File: src/main/resources/templates/typescript/test/package.json:9-11
Timestamp: 2024-11-12T12:51:40.391Z
Learning: In the Artemis TypeScript programming exercise templates, the "assignment" workspace directory specified in `src/main/resources/templates/typescript/test/package.json` is copied in by the CI beforehand.
Learnt from: magaupp
PR: ls1intum/Artemis#9440
File: src/main/resources/templates/typescript/solution/package.json:4-7
Timestamp: 2024-11-12T12:51:58.050Z
Learning: In the Artemis TypeScript programming exercise templates, students do not run the tests, so the `package.json` files in the solution and exercise directories should not include a `"test"` script.
Learnt from: magaupp
PR: ls1intum/Artemis#9440
File: src/main/resources/templates/typescript/exercise/package.json:4-7
Timestamp: 2024-11-12T12:51:58.050Z
Learning: In TypeScript programming exercises in Artemis, tests are not run by the students, so it's unnecessary to include a test script in the package.json file.
Learnt from: magaupp
PR: ls1intum/Artemis#9440
File: src/main/resources/templates/typescript/test/package.json:4-8
Timestamp: 2024-11-12T12:51:58.050Z
Learning: In Artemis TypeScript programming exercise templates, the test `package.json` file is only used for testing and does not require a `start` script.
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.
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.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (15)
src/test/resources/test-data/static-code-analysis/expected/eslint.json (1)
1-27: New File Addition: Correct JSON Structure for ESLint ResultsThe new JSON file correctly provides a structured output for ESLint analysis. The file includes a "tool" key and an "issues" array with all the necessary properties (filePath, start/end lines and columns, rule, category, message, and priority). One suggestion is to verify that the absolute file paths (e.g.,
/var/tmp/testing-dir/assignment/src/client.js) align with your test environment requirements; if portability is a concern, consider using relative paths in your test setup.src/test/resources/test-data/static-code-analysis/reports/eslint.sarif (3)
1-3: LGTM! Valid SARIF schema configuration.The SARIF version and schema URL are correctly specified according to the standard format.
58-107: LGTM! Well-structured error reporting.The results section provides clear, actionable error messages with precise location information, which aligns well with the PR's objective of enabling static code analysis.
6-30:✅ Verification successful
Verify ESLint version and consider expanding the rule set.
The ESLint configuration looks good, but let's verify the version and consider expanding the rule set.
Consider adding more essential rules such as:
no-unused-varsno-undefno-consolesemiquotesThese rules would help catch more potential issues during static analysis.
🌐 Web query:
What is the latest version of ESLint and are there any known security vulnerabilities in version 9.20.1?💡 Result:
The latest version of ESLint is v9.20.1, released on February 11, 2025[2][7]. According to Snyk's vulnerability database:
- No direct vulnerabilities have been reported for ESLint v9.20.1[2]
- The related
@eslint/jspackage also shows no security vulnerabilities[3]Recent security scans indicate:
- No CVEs or exploit patterns identified in versions ≥9.14.0 through current[2]
- Stable release channel shows clean security status since November 2024[2][10]
For context:
- The ESLint team maintains a strict semantic versioning policy with security-focused release practices[7]
- Version 9.20.x series includes critical fixes for configuration handling and type safety improvements[4]
Always verify dependencies through your project's vulnerability scanning tools, but current data shows v9.20.1 as secure[2][3][7].
Citations:
- 1: https://eslint.org/blog/2024/11/eslint-v9.14.0-released/
- 2: https://security.snyk.io/package/npm/eslint
- 3: https://security.snyk.io/package/npm/@eslint%2Fjs
- 4: https://eslint.org/blog/2025/02/eslint-v9.20.0-released/
- 5: https://www.codiga.io/blog/eslint-software-security/
- 6: https://github.com/eslint/eslint/blob/main/package.json
- 7: https://www.npmjs.com/package/eslint
- 8: https://security.snyk.io/package/npm/eslint-plugin-vue
- 9: https://community.cisco.com/t5/network-security/asa-5525-running-9-16-4-cve-2024-20353-and-cve-2024-20359/td-p/5092822
- 10: https://github.com/eslint/eslint/releases
ESLint configuration verified; consider extending rule set (optional improvement).
The ESLint configuration in
src/test/resources/test-data/static-code-analysis/reports/eslint.sarifis confirmed to use version 9.20.1, which is the latest release and free of known security vulnerabilities. The current rule set (coveringno-debuggerandno-empty) is valid, but for broader static analysis it might be beneficial to add additional rules. For instance, consider including:
no-unused-varsno-undefno-consolesemiquotesThese enhancements could help capture more potential issues during static analysis.
src/main/resources/templates/typescript/staticCodeAnalysis/test/eslint.config.mjs (1)
10-11: LGTM! Good use of recommended configurations.The configuration correctly incorporates recommended settings from both JavaScript and TypeScript ESLint plugins.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/sarif/SingleCategoryCategorizer.java (1)
8-19: LGTM! Clean and focused implementation.The implementation follows SOLID principles with a single responsibility and immutable state.
src/main/java/de/tum/cit/aet/artemis/programming/domain/StaticCodeAnalysisTool.java (1)
20-20: LGTM! Consistent integration of ESLint.The changes correctly:
- Add ESLint with its SARIF output file pattern
- Map JavaScript to ESLint in the tools mapping
- Maintain alphabetical ordering
Also applies to: 37-37
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/scaparser/strategy/ParserPolicy.java (1)
20-23: LGTM! Well-integrated ESLint support.The changes:
- Add a well-documented generic category constant
- Correctly integrate ESLint using the SARIF parser
- Follow the established pattern for other SARIF-based tools
Also applies to: 41-41
src/test/java/de/tum/cit/aet/artemis/programming/StaticCodeAnalysisParserUnitTest.java (1)
66-69: LGTM!The test method follows the established pattern in the test class, using the correct helper method and file names. The test is properly annotated and handles exceptions correctly.
src/main/java/de/tum/cit/aet/artemis/programming/service/localci/LocalCIProgrammingLanguageFeatureService.java (1)
81-81: LGTM!The TypeScript configuration correctly enables static code analysis and plagiarism checks, aligning with the documented features in the programming exercise features documentation.
src/main/java/de/tum/cit/aet/artemis/core/config/StaticCodeAnalysisConfigurer.java (1)
98-99: LGTM!The TypeScript configuration correctly uses ESLint with a generic lint category, following the same pattern as other single-tool language configurations.
docs/user/exercises/programming-exercise-features.inc (1)
96-96: LGTM!The TypeScript feature documentation accurately reflects the implementation in the code, correctly indicating that static code analysis is supported only in Local CI.
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 similar tools in the switch statement.
src/main/resources/templates/aeolus/typescript/default_static.sh (1)
11-11:❓ Verification inconclusive
Review error handling in static code analysis.
The
|| [ $? -eq 1 ]condition allows the script to continue if ESLint exits with code 1. This might mask legitimate errors. Consider capturing and logging the exit code for better error tracking.
🏁 Script executed:
#!/bin/bash # Description: Check ESLint exit codes documentation # Test: Search for ESLint exit codes documentation rg -A 5 'exit codes|exit status' eslint.sarifLength of output: 100
Action Required: Verify Static Analysis Error Handling
The current condition
npm run lint:ci || [ $? -eq 1 ]allows the script to continue if the lint command exits with code 1. However, this approach may mask other legitimate errors. The earlier attempt to verify ESLint exit code documentation using a search in
eslint.sarifdid not produce any meaningful output. Please manually verify whether an exit code of 1 is an acceptable result for your use case and whether additional logging of the exit code might be beneficial for error tracking.
- Suggestion: Consider capturing the exit code in a variable (e.g.,
status=$?) immediately after executingnpm run lint:ci, log this value, and then decide conditionally how to proceed. This ensures that unintended non-1 errors aren’t silently ignored.src/main/resources/templates/typescript/staticCodeAnalysis/test/package.json (1)
11-13:✅ Verification successful
Verify workspace variable resolution.
The
${studentParentWorkingDirectoryName}variable is used in both scripts and workspaces. Ensure this variable is properly resolved during CI execution.
🏁 Script executed:
#!/bin/bash # Description: Check if the variable is defined and resolved # Test: Search for variable definition or usage rg -A 5 'studentParentWorkingDirectoryName'Length of output: 41963
CI Variable Resolution Verified
The grep results confirm that the
${studentParentWorkingDirectoryName}placeholder is defined (insrc/main/java/de/tum/cit/aet/artemis/core/config/Constants.java) and is consistently used across multiple configuration files and scripts—including the workspaces array insrc/main/resources/templates/typescript/staticCodeAnalysis/test/package.json. As long as your CI configuration replaces these placeholders with actual values at runtime, everything appears to be in order.
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:
no-empty-object-typecode issueEdit 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