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

Location#compareTo(Location) breaks with undefined Location #3518

Closed
BookWood7th opened this issue Oct 17, 2024 · 4 comments
Closed

Location#compareTo(Location) breaks with undefined Location #3518

BookWood7th opened this issue Oct 17, 2024 · 4 comments
Assignees
Labels

Comments

@BookWood7th
Copy link
Contributor

Description

The Location class may be instantiated without an associated file. This causes fileUri to be null. Due to this Location::compareTo causes a NullPointerException, when used by a Location without an associated file

Reproducible

always

Steps to reproduce

Describe the steps needed to reproduce the issue.

  1. Create a Location without an associated fileUri (Location.UNDEFINED / any constructor values with fileUri set to null)
  2. Create a second Location object
  3. call compareTo on the first Location comparing it to the second

Expected to sort by their Position objects. Actually throws NullPointerException.

Additional information


@BookWood7th BookWood7th changed the title Bug in compareTo implementation for Locations IssueDialog with PositionedStrings with undefined Locations Oct 17, 2024
@BookWood7th
Copy link
Contributor Author

BookWood7th commented Oct 17, 2024

The issue actually does not concern Location but instead IssueDialog where sorting is done without checking for these NullPointerExceptions. May also be an edge case of Java documentation for Array.sort, which can't take Comparators that don't accept null values

@mattulbrich
Copy link
Member

Well, compareTo should not fail for valid argument objects, so I'd say it is a bug in the comparison.

Probably easy to fix by making the comparison null-aware, isn't it?

@BookWood7th
Copy link
Contributor Author

BookWood7th commented Oct 21, 2024

The issue is reasonably easy to fix and I believe I found a somewhat elegant solution to this.
The proposed solution adds explicit nullsLast comparators that check for null values of fileUri and position fields in Location before using the respective natural order of these fields in Location#compareTo(Location)

@BookWood7th BookWood7th changed the title IssueDialog with PositionedStrings with undefined Locations Location#compareTo(Location) breaks with undefined Location Oct 23, 2024
@BookWood7th
Copy link
Contributor Author

Was fixed in #3519

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants