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

Fixes for stale, misattributed, and incorrect diagnostics #6

Merged
merged 6 commits into from
Nov 25, 2021

Conversation

keynmol
Copy link
Collaborator

@keynmol keynmol commented Nov 24, 2021

Issue #, if available:

Description of changes:

  1. Publish diagnostics for files that have failures in the model (not just files that triggered failed compilation)
  2. Explicitly publish empty list of diagnostics for files that have no errors (to clear out between compilation)
  3. Never publish diagnostics for temporary files we create during didChange. Instead:
    • Massage a list of validation events, to change the locations of diagnostics to point at real workspace files
    • Don't persist the model (this.project) if we're not working with real files (as in the case of didOpen and didSave). This prevents stale diagnostics from accumulating with every change until the user presses Save.
  4. Add tests for all this nightmare.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@@ -187,33 +190,35 @@ public void didChange(DidChangeTextDocumentParams params) {
File tempFile = null;

try {
tempFile = File.createTempFile("smithy", SmithyProject.SMITHY_EXTENSION);
if (params.getContentChanges().size() > 0) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this, tempFile would be set before .get(0) blows up, and Option.ofNullable will not be reliable (it will have a filename, but the file will be empty)

Either<Exception, SmithyProject> loadedModel = this.project.recompile(path);
String changedFileUri = original.map(File::getAbsolutePath).orElse(path.getAbsolutePath());

public void report(Either<String, List<PublishDiagnosticsParams>> result) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Reporting and recompilation are now separate, to aid with testing and general declutter.

@Baccata Baccata merged commit 354c0c3 into dss Nov 25, 2021
@Baccata Baccata deleted the fix-stale-and-tempfile-diagnostics branch November 25, 2021 13:12
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.

None yet

2 participants