-
Notifications
You must be signed in to change notification settings - Fork 1
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
278 provide a means to detect where behaviour changes may occur due to new handling of private methods overrides in v61 #279
278 provide a means to detect where behaviour changes may occur due to new handling of private methods overrides in v61 #279
Conversation
private def findPrivateShadow(method: ApexMethodLike): Option[ApexMethodLike] = { | ||
val shadows = method.shadows.collect { case m: ApexMethodLike => m } | ||
val privateShadow = shadows.find(_.visibility == PRIVATE_MODIFIER) | ||
privateShadow.orElse(shadows.collectFirst(Function.unlift(findPrivateShadow))) |
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.
Is this doing transitives, like e.g. protected -> protected -> private
?
Not seen Function.unlift
before either.
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.
Yep, for transitives, not sure we have many but thought best test for them. Think this is first time I have done this, really just a find-first function but works nicely with Option. method.shadows is slightly odd here in that it's a Set which I guess is to allow for class & interface shadowing at same time.
Diagnostic( | ||
ERROR_CATEGORY, | ||
method.idLocation, | ||
s"The overrides of this private method will fail in v61, see $overrides" |
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.
Nice, I will say though my preference would be to keep ignoring API versions and say things like "this private is overridden, increase visibility to protected/public" / "cannot override private method, it is already defined here" etc.
I okay with it like it is, just wanted to make note
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.
Was thinking when we change MethodMap to issue warnings we can re-work this to improve. Just wanted to make sure people understood this error is related to recent API change for now.
This adds errors to both private base method and super methods if we see shadowing relationships. I have tested perf on FDN and looks to be negligible. Think maybe could do with some more test cases though but will let you take a look.