Skip to content

Conversation

Napalys
Copy link
Contributor

@Napalys Napalys commented Mar 12, 2025

Fixes extractor error discovered in #18899 (comment)

@github-actions github-actions bot added the JS label Mar 12, 2025
@Napalys Napalys force-pushed the js/extractor_error_handler branch from 5c50f61 to 3cdd34d Compare March 12, 2025 08:47
@Napalys Napalys requested a review from asgerf March 12, 2025 09:50
@Napalys Napalys force-pushed the js/extractor_error_handler branch from 3cdd34d to c3ecf70 Compare March 12, 2025 10:35
@asgerf
Copy link
Contributor

asgerf commented Mar 13, 2025

In TypeExprKind.getTypeExprKind could you try adding a case for ParenthesizedExpression (similar to ParenthesizedTypeExpr):

@Override
public Integer visit(ParenthesizedExpression nd, Void c) {
  return parenthesizedTypeExpr;
}

@Napalys Napalys force-pushed the js/extractor_error_handler branch 3 times, most recently from 09f41bc to 6d3761f Compare March 14, 2025 12:24
@Napalys Napalys force-pushed the js/extractor_error_handler branch from 6d3761f to 1468e81 Compare March 14, 2025 12:42
@Napalys Napalys marked this pull request as ready for review March 14, 2025 13:31
@Copilot Copilot AI review requested due to automatic review settings March 14, 2025 13:31
@Napalys Napalys requested a review from a team as a code owner March 14, 2025 13:31
Copy link
Contributor

@Copilot 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 updates the extractor to handle errors gracefully by introducing validity checks in type expression conversions. Key changes include:

  • Adding test files with invalid extends to trigger extractor error handling.
  • Introducing a default method isValidExpression in ITypeExpression.
  • Implementing an override of isValidExpression in MemberExpression.
  • Modifying TypeScriptASTConverter to use isValidExpression in type conversion.

Reviewed Changes

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

File Description
javascript/extractor/tests/ts/input/invalidExtends.ts Adds test cases with invalid "extends" constructs for extractor error handling
javascript/extractor/src/com/semmle/ts/ast/ITypeExpression.java Adds default validity check method in the ITypeExpression interface
javascript/extractor/src/com/semmle/js/ast/MemberExpression.java Adds an override for isValidExpression to check allowed object types
javascript/extractor/src/com/semmle/ts/extractor/TypeScriptASTConverter.java Updates the asType method to apply the validity check from ITypeExpression
Files not reviewed (2)
  • javascript/ql/test/library-tests/TypeScript/Types/printAst.expected: Language not supported
  • javascript/ql/test/library-tests/TypeScript/Types/tests.expected: Language not supported

Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more

…ll for type validation.

Co-authored-by: Asgerf <asgerf@github.com>
@Napalys Napalys requested a review from asgerf March 14, 2025 14:00
Copy link
Contributor

@asgerf asgerf left a comment

Choose a reason for hiding this comment

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

👍

@Napalys Napalys merged commit 9134f79 into github:main Mar 17, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants