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

Better design of the CodeAndLocationProvider #1414

Merged
merged 4 commits into from
Jan 16, 2024

Conversation

oxisto
Copy link
Member

@oxisto oxisto commented Jan 16, 2024

Instead of directly setting the code and location provider in the implementation of the CodeAndLocationProvider we rather now have an implementor of this interface implement the methods codeOf and locationOf and set the code/location in the node builder. This is more in line with the other providers, since they should provide a value, but not set it.

Instead of directly setting the code and location provider in the implementation of the `CodeAndLocationProvider` we rather now have an implementor of this interface implement the methods `codeOf` and `locationOf` and set the code/location in the node builder. This is more in line with the other providers, since they should *provide* a value, but not *set* it.

The function `setCodeAndLocation` will stay for now for legacy reason and is marked deprecated. We seem to have a lot of language frontends that directly call this function, which is bad design anyway. One should supply the raw node to the node builder instead.
@oxisto oxisto force-pushed the better-location-provider branch from 630db6b to e0a6d48 Compare January 16, 2024 10:17
Copy link

sonarcloud bot commented Jan 16, 2024

@oxisto oxisto marked this pull request as ready for review January 16, 2024 11:42
@konradweiss konradweiss merged commit 36a9d0e into main Jan 16, 2024
4 checks passed
@konradweiss konradweiss deleted the better-location-provider branch January 16, 2024 14:33
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.

2 participants