Skip to content
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 the notion of equivalence #158

Closed
Ladicek opened this issue Nov 3, 2021 · 2 comments · Fixed by #173
Closed

Add the notion of equivalence #158

Ladicek opened this issue Nov 3, 2021 · 2 comments · Fixed by #173
Assignees
Milestone

Comments

@Ladicek
Copy link
Collaborator

Ladicek commented Nov 3, 2021

Jandex classes often don't have equals at all, or have equals that isn't necessarily useful. That makes perfect sense e.g. in case of annotation overlaying, where you would have e.g. two ClassInfo objects for the same class, each having a different set of annotations. These ClassInfo objects should not be considered equal, but they should be considered equivalent.

I can see several ways how equivalence could be introduced to Jandex. I'm currently thinking it should be completely separate from the AnnotationTarget hierarchy. Something like this:

public class EquivalenceKey {
    public static EquivalenceKey of(ClassInfo) { ... }
    public static EquivalenceKey of(MethodInfo) { ... }
    public static EquivalenceKey of(MethodParameterInfo) { ... }
    public static EquivalenceKey of(FieldInfo) { ... }

    // guaranteed to have `equals` and `hashCode` (and `toString`)
    // everything else would be private
}
@Ladicek Ladicek added this to the 3.0.0 milestone Nov 3, 2021
@Ladicek Ladicek self-assigned this Nov 3, 2021
@Ladicek Ladicek changed the title add notion of equivalence Add notion of equivalence Nov 3, 2021
@Ladicek Ladicek changed the title Add notion of equivalence Add the notion of equivalence Nov 3, 2021
@Ladicek
Copy link
Collaborator Author

Ladicek commented Nov 3, 2021

I've already implemented this, but it needs more thinking (and also adding tests and documentation, the usual). In my current implementation, equivalence is established like this:

  • for classes: based on the name
  • for methods: based on the name of the declaring class, the name of the method, and the list of parameter types
  • for method parameters: based on the method equivalence key and the parameter index
  • for fields: based on the name of the declaring class and the name of the field

I'm pretty confident it's fine for classes and fields (and method parameters, because they delegate to method equivalence). For methods, return type most likely needs to be considered too (as I said, more thinking).

@Ladicek Ladicek linked a pull request Feb 14, 2022 that will close this issue
@Ladicek
Copy link
Collaborator Author

Ladicek commented Mar 1, 2022

Done in #173.

@Ladicek Ladicek closed this as completed Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant