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

Fix problems reporting errors between multiple files #1224

Merged
merged 10 commits into from
Oct 20, 2023

Conversation

bugarela
Copy link
Collaborator

@bugarela bugarela commented Oct 19, 2023

Hello :octocat:

This fixes #1199 and fixes #1174 both on CLI and language server.

Instead of keeping the source code as a string, we have to keep it as a map from filenames to file content. We then use this map to properly report errors.

  • Tests added for any new code
  • Documentation added for any new functionality
  • Entries added to the respective CHANGELOG.md for any new functionality
  • Feature table on README.md updated for any listed functionality

@bugarela bugarela marked this pull request as ready for review October 19, 2023 17:49
@bugarela bugarela requested review from thpani and konnov October 19, 2023 17:49
Copy link
Contributor

@konnov konnov left a comment

Choose a reason for hiding this comment

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

Looks good! Just left a few comments.

@@ -625,10 +629,21 @@ function printErrorMessages(
const modulesText = state.moduleHist + inputText
const messages = errors.map(mkErrorMessage(state.compilationState.sourceMap))
// display the error messages and highlight the error places
// FIXME: moudulesText can come from multiple files, but `compileFromCode` ignores that.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe open an issue on that?

@@ -276,7 +279,9 @@ export function compileFromCode(
rand: (bound: bigint) => bigint
): CompilationContext {
// parse the module text
const { modules, table, sourceMap, errors } = parse(idGen, '<module_input>', mainPath, code)
// FIXME: We should build a proper sourceCode map from the files we previously loaded
Copy link
Contributor

Choose a reason for hiding this comment

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

also open an issue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both should be fixed with #1052, but I'll make sure to add this additional requirement there!

@bugarela bugarela enabled auto-merge October 20, 2023 14:36
@bugarela bugarela merged commit 50140d9 into main Oct 20, 2023
15 checks passed
@bugarela bugarela deleted the gabriela/error-reporting-between-files branch October 20, 2023 15:09
This was referenced Nov 8, 2023
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.

Error in error reporting by the type checker Errors in one file can be reported in other file
2 participants