Skip to content

Java: Fix ClassInstanceExpr::isDiamond not working for anonymous classes - #15429

Open
Marcono1234 wants to merge 2 commits into
github:mainfrom
Marcono1234:marcono1234/anonymous-isDiamond
Open

Marcono1234 wants to merge 2 commits into
github:mainfrom
Marcono1234:marcono1234/anonymous-isDiamond

Conversation

@Marcono1234

Copy link
Copy Markdown
Contributor

Previously it was only working for non anonymous classes because it required that getType() instanceof ParameterizedClass which does not hold for anonymous classes.

@Marcono1234 Marcono1234 Jan 24, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Possibly a bit irritating that the test is under /java7/ but now requires Java 9. But is this fine nonetheless since the diamond syntax was introduced in Java 7 (but extended in Java 9)?

this.getAnonymousClass().getASupertype() instanceof ParameterizedType and
// Ignore Kotlin code; otherwise seems to erroneously match `object` expression with explicit
// type arguments, possibly because extractor is not properly extracting the type args?
not this.getCompilationUnit().isKotlinSourceFile()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@igfoo is there a quick fix for the Kotlin case rather than hack Kotlin source out like this? @Marcono1234 for clarity could you show a Kotlin snippet that otherwise behaves badly?

@Marcono1234 Marcono1234 Feb 19, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If I omit that exclusion of Kotlin source files and then run the following query on the database for https://github.com/Kotlin/kotlinx.coroutines (latest database from GitHub was built with CodeQL CLI 2.16.1)

import java

from ClassInstanceExpr e
where
  e.isDiamond() and
  e.getCompilationUnit().isKotlinSourceFile()
select e

it finds for example this expression with an explicit type argument:
https://github.com/Kotlin/kotlinx.coroutines/blob/8c516f5ab1fcc39629d2838489598135eedd7b80/kotlinx-coroutines-core/common/test/AbstractCoroutineTest.kt#L13C25-L13C59

Though maybe the isDiamond predicate does not make sense for Kotlin in the first place; at least there is (to my knowledge) no <> syntax but instead the type arguments are simply omitted if they can be inferred by the compiler.
Or at least the name isDiamond might be a bit confusing then.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep, that looks like a bug in our Kotlin support at the moment. Excluding like this looks fine for now. I've made an internal ticket to remind us to come back to this.

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.

4 participants