Skip to content

Kotlin: Enable java/misnamed-type query #11436

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

Merged
merged 4 commits into from
Nov 29, 2022

Conversation

igfoo
Copy link
Contributor

@igfoo igfoo commented Nov 25, 2022

We used to get alerts for the class around a local function, a lambda, or a function reference, which we give name "". Now those are marked as compiler-generated, and the query ignores compiler-generated types.

We used to get alerts for the class around a local function, a lambda,
or a function reference, which we give name "". Now those are marked as
compiler-generated, and the query ignores compiler-generated types.
@igfoo igfoo force-pushed the igfoo/NamingConventionsRefTypes branch from efe9744 to a423f5f Compare November 25, 2022 17:11
@igfoo igfoo marked this pull request as draft November 25, 2022 20:27
@igfoo igfoo marked this pull request as ready for review November 28, 2022 15:23
private fun extractGeneratedClass(
localFunction: IrFunction,
superTypes: List<IrType>,
compilerGeneratedKind: CompilerGeneratedKinds? = null
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a case when extractGeneratedClass is called and the class should be non compiler generated? Should this parameter be nullable?

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 assume that any class extracted by "extractGeneratedClass" should be treated as compiler-generated. As in the other comment, null here really means "the default".

val compilerGeneratedKind = if (s.origin == IrDeclarationOrigin.ADAPTER_FOR_CALLABLE_REFERENCE) {
CompilerGeneratedKinds.DECLARING_CLASSES_OF_ADAPTER_FUNCTIONS
} else {
null
Copy link
Contributor

Choose a reason for hiding this comment

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

It feels unintuitive for me that we pass null, but in the end it becomes CompilerGeneratedKinds.CALLABLE_CLASS. Should we instead explicitly specify the latter here?

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 considered that, but then we'd have to repeat CALLABLE_CLASS in 3 places. I think of the null here as "no special value; just use the default". Perhaps I should have called it compilerGeneratedKindOverride instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. Another option would be specifying a default parameter value. But compilerGeneratedKindOverride also sounds reasonable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem with a default parameter value is I think I'd need to specify it for both overloads of the function, as well as passing it as a parameter in the call that makes use of the "last argument is a block" syntax. So I've gone with the ...Override name to avoid repeating it.

@igfoo igfoo merged commit 7eaef0c into github:main Nov 29, 2022
@igfoo igfoo deleted the igfoo/NamingConventionsRefTypes branch November 29, 2022 18:45
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