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

Run elm-review in a custom watch mode #942

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,308 changes: 1,183 additions & 125 deletions package-lock.json

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
},
"dependencies": {
"chokidar": "^3.5.3",
"elm-review": "^2.9.2",
"escape-string-regexp": "^4.0.0",
"execa": "^5.1.1",
"fast-diff": "^1.2.0",
Expand Down
3 changes: 3 additions & 0 deletions src/providers/astProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,12 @@ export class ASTProvider {

private treeChangeEvent = new Emitter<{
sourceFile: ISourceFile;
newText: string;
declaration?: SyntaxNode;
}>();
readonly onTreeChange: Event<{
sourceFile: ISourceFile;
newText: string;
declaration?: SyntaxNode;
}> = this.treeChangeEvent.event;

Expand Down Expand Up @@ -194,6 +196,7 @@ export class ASTProvider {
if (tree) {
this.treeChangeEvent.fire({
sourceFile,
newText,
declaration: changedDeclaration,
});
}
Expand Down
82 changes: 47 additions & 35 deletions src/providers/diagnostics/diagnosticsProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
Diagnostic as LspDiagnostic,
DiagnosticSeverity,
FileChangeType,
DidChangeTextDocumentParams,
} from "vscode-languageserver";
import { TextDocument } from "vscode-languageserver-textdocument";
import { URI } from "vscode-uri";
Expand All @@ -26,7 +25,6 @@ import { ElmMakeDiagnostics } from "./elmMakeDiagnostics";
import { DiagnosticKind, FileDiagnostics } from "./fileDiagnostics";
import { ISourceFile } from "../../compiler/forest";
import { ElmReviewDiagnostics } from "./elmReviewDiagnostics";
import { IElmAnalyseJsonService } from "./elmAnalyseJsonService";

export interface IElmIssueRegion {
start: { line: number; column: number };
Expand Down Expand Up @@ -100,12 +98,11 @@ export class DiagnosticsProvider {

private pendingRequest: DiagnosticsRequest | undefined;
private pendingDiagnostics: PendingDiagnostics;
private diagnosticsDelayer: Delayer<any>;
private pendingCompilerDiagnostics: Set<string>;
private diagnosticsDelayer: Delayer<unknown>;
private diagnosticsOperation: MultistepOperation;
private changeSeq = 0;

private elmAnalyseJsonService: IElmAnalyseJsonService;

constructor() {
this.clientSettings = container.resolve("ClientSettings");

Expand All @@ -121,14 +118,11 @@ export class DiagnosticsProvider {

this.workspaces = container.resolve("ElmWorkspaces");

this.elmAnalyseJsonService = container.resolve<IElmAnalyseJsonService>(
"ElmAnalyseJsonService",
);

const astProvider = container.resolve(ASTProvider);

this.currentDiagnostics = new Map<string, FileDiagnostics>();
this.pendingDiagnostics = new PendingDiagnostics();
this.pendingCompilerDiagnostics = new Set<string>();
this.diagnosticsDelayer = new Delayer(300);

const clientInitiatedDiagnostics =
Expand All @@ -148,15 +142,7 @@ export class DiagnosticsProvider {
return;
}

void this.getElmMakeDiagnostics(sourceFile).then((hasElmMakeErrors) => {
if (hasElmMakeErrors) {
this.currentDiagnostics.forEach((_, uri) => {
this.updateDiagnostics(uri, DiagnosticKind.ElmReview, []);
});
} else {
void this.getElmReviewDiagnostics(sourceFile);
}
});
void this.getElmMakeDiagnostics(sourceFile);

// If we aren't doing them on change, we need to trigger them here
if (disableDiagnosticsOnChange) {
Expand Down Expand Up @@ -218,39 +204,30 @@ export class DiagnosticsProvider {
this.requestAllDiagnostics();
}

// eslint-disable-next-line @typescript-eslint/no-unused-vars
astProvider.onTreeChange(({ sourceFile, declaration }) => {
astProvider.onTreeChange(({ sourceFile }) => {
if (!clientInitiatedDiagnostics && !disableDiagnosticsOnChange) {
this.requestDiagnostics(sourceFile.uri);
}
});

this.documentEvents.on("change", (params: DidChangeTextDocumentParams) => {
this.documentEvents.on("change", () => {
this.change();

this.updateDiagnostics(
params.textDocument.uri,
DiagnosticKind.ElmReview,
[],
);

// We need to cancel the request as soon as possible
if (!clientInitiatedDiagnostics && !disableDiagnosticsOnChange) {
if (this.pendingRequest) {
if (this.pendingRequest && !this.pendingRequest.isCancelled) {
this.pendingRequest.cancel();
this.pendingRequest = undefined;
}
}
});
}

public interruptDiagnostics<T>(f: () => T): T {
if (!this.pendingRequest) {
if (!this.pendingRequest || this.pendingRequest.isCancelled) {
return f();
}

this.pendingRequest.cancel();
this.pendingRequest = undefined;
const result = f();

this.triggerDiagnostics();
Expand Down Expand Up @@ -284,6 +261,7 @@ export class DiagnosticsProvider {

private requestDiagnostics(uri: string): void {
this.pendingDiagnostics.set(uri, Date.now());
this.pendingCompilerDiagnostics.add(uri);
this.triggerDiagnostics();
}

Expand All @@ -296,14 +274,18 @@ export class DiagnosticsProvider {
program.getForest().treeMap.forEach(({ uri, writeable }) => {
if (writeable) {
this.pendingDiagnostics.set(uri, Date.now());
this.pendingCompilerDiagnostics.add(uri);
}
});
});

this.triggerDiagnostics();
}

private triggerDiagnostics(delay = 200): void {
const sendPendingDiagnostics = (): void => {
this.elmReviewDiagnostics.startDiagnostics();

const orderedFiles = this.pendingDiagnostics.getOrderedFiles();

if (this.pendingRequest) {
Expand Down Expand Up @@ -333,6 +315,7 @@ export class DiagnosticsProvider {
() => {
if (request === this.pendingRequest) {
this.pendingRequest = undefined;
this.pendingCompilerDiagnostics.clear();
}
},
));
Expand Down Expand Up @@ -385,6 +368,7 @@ export class DiagnosticsProvider {
cancellationToken: CancellationToken,
): Promise<void> {
const followMs = Math.min(delay, 200);

const serverCancellationToken = new ServerCancellationToken(
cancellationToken,
);
Expand Down Expand Up @@ -450,6 +434,18 @@ export class DiagnosticsProvider {
diagnostics.map(convertFromCompilerDiagnostic),
);

if (!this.pendingDiagnostics.has(uri)) {
this.pendingCompilerDiagnostics.delete(uri);
}

if (this.hasSyntacticOrSemanticErrors()) {
this.updateDiagnostics(uri, DiagnosticKind.ElmReview, []);
}

if (this.pendingCompilerDiagnostics.size === 0) {
void this.triggerElmReviewDiagnostics();
}

next.immediate(() => {
this.updateDiagnostics(
uri,
Expand Down Expand Up @@ -528,11 +524,27 @@ export class DiagnosticsProvider {
);
}

private async getElmReviewDiagnostics(
sourceFile: ISourceFile,
): Promise<void> {
private async triggerElmReviewDiagnostics(): Promise<void> {
// Get elm-review diagnostics if there are no errors
if (!this.hasSyntacticOrSemanticErrors()) {
await this.getElmReviewDiagnostics();
}
}

private hasSyntacticOrSemanticErrors(): boolean {
let errorCount = 0;
this.currentDiagnostics.forEach((diagnostics) => {
errorCount += diagnostics
.getForKind(DiagnosticKind.Syntactic)
.concat(diagnostics.getForKind(DiagnosticKind.Semantic))
.filter((d) => d.severity === DiagnosticSeverity.Error).length;
});
return errorCount > 0;
}

private async getElmReviewDiagnostics(): Promise<void> {
const elmReviewDiagnostics =
await this.elmReviewDiagnostics.createDiagnostics(sourceFile);
await this.elmReviewDiagnostics.createDiagnostics();

// remove old elm-review diagnostics
this.resetDiagnostics(elmReviewDiagnostics, DiagnosticKind.ElmReview);
Expand Down
9 changes: 9 additions & 0 deletions src/providers/diagnostics/diagnosticsRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ export class DiagnosticsRequest {
if (this.done) {
return;
}

if (this.isCancelled) {
return;
}

this.done = true;
onDone();
});
Expand All @@ -50,4 +55,8 @@ export class DiagnosticsRequest {

this.token.dispose();
}

public get isCancelled(): boolean {
return this.token.token.isCancellationRequested;
}
}
Loading