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

Group dumped declaration by declaration kind (class, function, property, etc.) #224

Merged
merged 6 commits into from
Jun 10, 2024

Conversation

fzhinkin
Copy link
Collaborator

@fzhinkin fzhinkin commented May 6, 2024

Currently, all dumped declarations are sorted lexicographically, potentially by mixing different kinds of declarations. That approach differs from what we do for JVM dumps, where declarations are grouped by a declaration kind.

With this change, declarations will be ordered as follows:

  • Firstly, by how common are they w.r.t. set of targets: more common declarations will be printed first, followed by less common ones;
  • If two declarations share the same set of targets, they will be compared by their kind (class, function, property, etc.): depending on the context (top-level declaration vs nested), the order will be chosen using rules listed below;
  • Finally, if two declarations share the same target sets and are of the same kind, we'll compare them lexicographically.

This PR introduces the following order:

  • For top-level declarations:
    • annotation
    • enum class
    • interface
    • regular class
    • object
    • const val
    • val
    • var
    • functions
    • everything else
  • For nested declarations:
    • constructor
    • enum entry
    • const val
    • val
    • var
    • function
    • annotation
    • enum class
    • interface
    • regular class
    • object

Closes #197

@fzhinkin fzhinkin requested a review from ilya-g May 6, 2024 09:30
@fzhinkin fzhinkin linked an issue May 6, 2024 that may be closed by this pull request
@fzhinkin fzhinkin force-pushed the 197-consider-decl-kind-when-sorting-dump branch from a9a5b5a to 41770eb Compare May 27, 2024 08:30
@ilya-g
Copy link
Member

ilya-g commented Jun 10, 2024

I have a feeling that the sorting test doesn't specifically test sorting inside groups. For example, try replacing lhs.text.compareTo(rhs.text) with 0 and see if there will be failures to sort declarations inside groups.

@fzhinkin
Copy link
Collaborator Author

@ilya-g using a fixed value instead of lhs.text.compareTo(rhs.text) leads to test failures (both unit- and functional-tests).

But I agree that a test explicitly covering ordering is missing, I'll add it.

@fzhinkin fzhinkin merged commit a835a73 into develop Jun 10, 2024
1 check passed
shanshin pushed a commit to JetBrains/kotlin that referenced this pull request Oct 28, 2024
shanshin pushed a commit to JetBrains/kotlin that referenced this pull request Dec 3, 2024
shanshin pushed a commit to JetBrains/kotlin that referenced this pull request Dec 13, 2024
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 this pull request may close these issues.

Klib .api files should sort members above types
2 participants