SourceIndex and compiler erros fileURI #128
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…uilder`. This commit upgrades workspace errors to `SourceCodeState` errors right from the get-go if `fileURI` is defined in the compilation error.
simerplaha
reviewed
Mar 11, 2024
Comment on lines
232
to
241
| def toWorkspaceDiagnostics(workspace: WorkspaceState.IsSourceAware): Iterable[FileDiagnostic] = { | ||
| workspace match { | ||
| case compiled: WorkspaceState.Errored => | ||
| // Group by fileURI provided errors | ||
| compiled.workspaceErrors groupBy(_.index.fileURI) map { | ||
| case (Some(fileURI), errors) => | ||
| // fileURI is known, we try to find the source code. | ||
| compiled.sourceCode.find(_.fileURI == fileURI) match { | ||
| case Some(source) => | ||
| // Source code is known, we report errors at source-code level. |
There was a problem hiding this comment.
This is great! Just a minor adjustment: we can move this logic to execute immediately after we compile the contracts. This way, compilation errors containing fileURI won't be transformed into workspaceErrors at all. So they will be recognised as regular errors associated with a file throughout the code from the get-go and we can test it everywhere, rather than only when responding to the client in diagnostics.
I've submitted PR #130 to this branch with the change and this is the function.
…RI` as function parameter.
simerplaha
reviewed
Mar 11, 2024
Comment on lines
12
to
15
| implicit val fileURI:java.net.URI = file | ||
|
|
||
| val statefulParser = new SourceFileStatefulParser()(Some(fileURI)) | ||
| import statefulParser._ |
There was a problem hiding this comment.
SourceFileStatefulParser 👍🏼 .
Just minor adjustment: We can remove implicit fileURI and just pass it around as a function parameter where needed.
Implemented in PR #131.
Transform compilation errors to `SourceCode` errors immediately, if `SourceIndex.fileURI` exists
Remove `implicit fileURI` and pass it as a function parameter.
Mandatory `fileURI` for internal `SourceIndex` related functions
Merged
Split `SourceCodeState.ErrorSource`
simerplaha
approved these changes
Mar 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Still waiting for a release from the node, but I think we could already ope the discussion.
From test and all my manual testing it seems to work fine, but as always I might miss some place.
The important change is in second commit.
As the
fileURIin errors are optional, it's good to keep all you work you did to have fileURI mandatory where we can.