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 missing -Yexplicit-nulls for presentation compiler #18776

Merged

Conversation

rochala
Copy link
Contributor

@rochala rochala commented Oct 27, 2023

With a new definition of untyped trees, it is now required to have -Yexplicit-nulls flag in modules that use them in order to have proper type checking. This PR adds the missing flag.

Without the flag, it was possible to first assign untyped trees to typed trees, and secondly use extension methods for typed trees which can be seen at KeywordsCompletions.scala with usage of untpdTree.filterSubtrees.

It is also blocked by: #18775

I can also make a workaround in the unmanaged module, but it will require a dependency on nightly version / a new release from metals.

@rochala rochala requested a review from tgodzik October 27, 2023 17:03
@rochala rochala force-pushed the add-explicit-null-to-presentation-compiler branch from 4da9ce8 to 7136850 Compare October 31, 2023 12:08
Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly very minor stuff, though I don't love that we need to do nn on everything coming from Java :/ Using project Lombok would help at some point?

@rochala
Copy link
Contributor Author

rochala commented Nov 6, 2023

Well, this is a tradeoff of having crashes from working on untyped trees as on typed ones, which are significantly harder to debug than simple null pointer assertion failure. They will also fail fast instead of some deeper place in the code. The cost of this is the nn or if you are sure that you won't be operating on trees, especially untyped, you can do an import instead. I don't know project Lombok, so I'm not sure if it can help, but adding another library for such small inconvenience is not worth it, at least in my opinion.

@rochala rochala force-pushed the add-explicit-null-to-presentation-compiler branch from 1ce3c5a to 1016d16 Compare November 6, 2023 16:47
@rochala rochala requested a review from tgodzik November 6, 2023 16:47
Copy link
Contributor

@tgodzik tgodzik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@rochala rochala merged commit e149e4c into scala:main Nov 8, 2023
18 checks passed
@Kordyjan Kordyjan added this to the 3.4.0 milestone Dec 20, 2023
@tgodzik tgodzik added the area:presentation-compiler Related to the presentation compiler module used by Metals and possibly other tools label May 8, 2024
WojciechMazur pushed a commit that referenced this pull request Jun 23, 2024
With a new definition of untyped trees, it is now required to have
`-Yexplicit-nulls` flag in modules that use them in order to have proper
type checking. This PR adds the missing flag.

Without the flag, it was possible to first assign untyped trees to typed
trees, and secondly use extension methods for typed trees which can be
seen at `KeywordsCompletions.scala` with usage of
`untpdTree.filterSubtrees`.

It is also blocked by: #18775

I can also make a workaround in the unmanaged module, but it will
require a dependency on nightly version / a new release from metals.
[Cherry-picked e149e4c]
WojciechMazur added a commit that referenced this pull request Jun 23, 2024
…LTS (#20758)

Backports #18776 to the LTS branch.

PR submitted by the release tooling.
[skip ci]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:presentation-compiler Related to the presentation compiler module used by Metals and possibly other tools
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants