[flutter_tools] Version.unknown is equivalent to Version(0,0,0) under compareTo, which can result in surprising behavior

[andrewkolos]

Directly related to #121468.

Filing this as a sub-issue to be fixed with its own PR (this also affects non Android Studio-related code).

Context

Version is a class that is used to represent a version of multiple pieces of software used throughout the tool, including Android Studio, IntelliJ, Visual Studio Code, and Cocoapods.

Issue

Version has a null-object¹ instantiation, Version.unknown, which is used in a few places to represent the flutter tool being unable to detect the version of some software. Because Version also implements Comparable, it is easy to write logic that compares a more typical version object (e.g. Version(1, 2, 3)) to Version.unknown, which doesn't semantically make sense. For a specific example, consider Version(0,0,0) == Version.unknown, which resolves to true.

Because of this, branching logic based on Version comparison is prone to error if Version.unknown is used. If someone uses Version.unknown and they compare Verison objects, they must remember to explicitly check for Version.unknown in addition to those comparisons. Otherwise, they are at risk of treating Version.unknown as a legitimate version (0.0.0). Again, see #121468.

Possible fixes

1. Remove Version.unknown entirely. The drawback is that tool code has to use something else to represent the inability to find/detect a version within a software installation. null is the easiest choice, but the meaning of null is not as clear as Version.unknown, and it results in slightly less readable code.
2. Make the compareTo function of Version aware of Version.unknown (which should be made into a static const). Specifically, comparison with Version.undefined should result in an exception being thrown. While arguably better than the current behavior, users still need to remember to check for Version.unknown before making comparisons.

I suggest option 1.

Footnotes

1. I'm abusing terminology here, since this object doesn't have any null/default behavior, but it still accomplishes something a null-object is typically used to do—avoiding references to null. ↩

[jonahwilliams]

Perhaps it should work more like a nan! 😅

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.