Skip to content

Conversation

@asgerf
Copy link
Contributor

@asgerf asgerf commented Apr 10, 2025

Standard JSON does not permit trailing commas, but tsconfig.json files rely on an extension of JSON with trailing commas and comments.

We already extract JSON using this extended format in general, but trailing commas in arrays were not supported, meaning tsconfig.json files were rejected in some cases.

@asgerf asgerf marked this pull request as ready for review April 10, 2025 06:52
Copilot AI review requested due to automatic review settings April 10, 2025 06:52
@asgerf asgerf requested a review from a team as a code owner April 10, 2025 06:52
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enables support for trailing commas in JSON arrays for tsconfig.json extraction.

  • Added a changenote documenting the fix.
  • Removed error-raising logic in JSONParser to allow trailing commas in array literals.
  • Updated the extractor version to reflect the changes.

Reviewed Changes

Copilot reviewed 3 out of 5 changed files in this pull request and generated no comments.

File Description
javascript/ql/src/change-notes/2025-04-10-json-array-trailing-comma.md Added changenote explaining the fix for trailing commas in array literals in tsconfig.json.
javascript/extractor/src/com/semmle/js/parser/JSONParser.java Removed the check that raised an error for omitted array elements, thereby allowing trailing commas.
javascript/extractor/src/com/semmle/js/extractor/Main.java Updated the extractor version to indicate the new release date.
Files not reviewed (2)
  • javascript/extractor/tests/json/input/array-trailing-comma.json: Language not supported
  • javascript/extractor/tests/json/output/trap/array-trailing-comma.json.trap: Language not supported

@asgerf asgerf merged commit d2a4f1e into github:main Apr 10, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants