-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Add kotlin test cases for lint module #460
Conversation
Tweak CallOnRequestPermissionsResultDetector to address onRequestPermissionsResult extension function.
private OnRequestPermissionsResultChecker(JavaContext context, UClass klass) { | ||
this.context = context; | ||
this.className = klass.getName(); | ||
isKotlin = context.getPsiFile() != null && "kotlin".equals(context.getPsiFile().getLanguage().getID()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shiraji I guess we can detect the file is kotlin or not as this!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's good to know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does getLanguage()
return an enum that already contains a Kotlin member? If so, and we have access to that, might as well use that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Actually Language
is not an Enum but just an abstract class.. 😅A bit unsmart but we gotta check ID 🤔
https://github.com/JetBrains/intellij-community/blob/571fbe96754e6c762fe8f7f8532ae2665bfbae61/platform/core-api/src/com/intellij/lang/Language.java#L42
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, it's OK then!
Can I merge? |
Sure! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup
Thank you guys! I'll release next version tonight! |
Issue
resolves #373
Overview