Skip to content

Java: Add CompilationUnit.getATypeInScope() #10498

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

Marcono1234
Copy link
Contributor

This predicate is mainly helpful for Javadoc queries and for queries which check whether the name of an element shadows another type.

Resolves #4119
It makes probably most sense to implement the logic requested there for CompilationUnit, as done in this PR because there it is also possible to account for shadowing between different kinds of imports (though this is currently not implemented by this PR).

This PR is a bit experimental and does not have a dedicated test; feedback is appreciated.

This predicate is mainly helpful for Javadoc queries and for queries which
check whether the name of an element shadows another type.
@Marcono1234 Marcono1234 force-pushed the marcono1234/compilation-unit-simple-name-type branch from ef7046a to 431aa2c Compare September 20, 2022 21:16
Comment on lines 47 to 73
or
exists(ImportStaticTypeMember importDecl |
importDecl.getCompilationUnit() = this and
result = importDecl.getATypeImport()
)
or
exists(ImportType importDecl |
importDecl.getCompilationUnit() = this and
result = importDecl.getImportedType()
)
or
exists(ImportStaticOnDemand importDecl |
importDecl.getCompilationUnit() = this and
result = importDecl.getATypeImport()
)
or
exists(ImportOnDemandFromType importDecl |
importDecl.getCompilationUnit() = this and
result = importDecl.getAnImport()
)
or
exists(ImportOnDemandFromPackage importDecl |
importDecl.getCompilationUnit() = this and
result = importDecl.getAnImport()
)
or
// From same package or java.lang, see https://docs.oracle.com/javase/specs/jls/se17/html/jls-7.html
Copy link
Contributor

Choose a reason for hiding this comment

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

Add CompilationUnit.getAnImport() to make this much briefer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean as separate predicate: getAnImport(): Import?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, so you don't need all the exists quantifiers but instead can do this.getAnImport().(ImportStaticOnDemand).getAnImport() for example

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have simplified the code but decided against a separate predicate getAnImport() for know because I am not sure if it would be useful in any other situations.
I hope that is fine.

* - The type is declared in the same package as this compilation unit
* - The type is declared in the package `java.lang`
*/
ClassOrInterface getATypeAvailableBySimpleName() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe AvailableBySimpleName -> InScope?

Copy link
Contributor Author

@Marcono1234 Marcono1234 Sep 22, 2022

Choose a reason for hiding this comment

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

In that case should the shadowing and simple name aspects be removed from the comments? Because a shadowed type would still be in scope (I think).

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know off hand if the JLS formally defines "in scope"? In either case though explicitly mentioning this tricky corner seems useful, so I'd keep it.

ClassOrInterface getATypeInScope() {
// See "Shadowing", https://docs.oracle.com/javase/specs/jls/se17/html/jls-6.html#jls-6.4.1
// Currently shadowing is not considered
result.(TopLevelType).getCompilationUnit() = this
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant w.r.t. the same-package test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right. Currently this structure matches (roughly?) the precedence order of shadowing, but I could drop this nonetheless if you want.

|
exception.getAnAncestor().hasQualifiedName("java.lang", uncheckedException)
)
predicate canThrow(Callable callable, Class exception) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question re: throwing things by an interface type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All exception types are classes though, so you cannot declare an interface type to be thrown, can you?
(neither in Kotlin, right?)

Copy link
Contributor

Choose a reason for hiding this comment

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

Doh right, I had Exception in mind as an interface. Never mind.

@Marcono1234 Marcono1234 changed the title Java: Add CompilationUnit.getATypeAvailableBySimpleName() Java: Add CompilationUnit.getATypeInScope() Sep 26, 2022
@aschackmull aschackmull merged commit b48b5d4 into github:main Sep 28, 2022
@Marcono1234 Marcono1234 deleted the marcono1234/compilation-unit-simple-name-type branch September 29, 2022 19:57
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.

Java: Add Import.getATypeImport
3 participants