-
Notifications
You must be signed in to change notification settings - Fork 13
Add more opportunities for cancellation for validate() #260
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
base: master
Are you sure you want to change the base?
Changes from 26 commits
981177f
67fb09a
ff221af
c0fc7c2
ecef8cd
a3ce0ee
77fe063
d2c6085
cebbf1d
6668ab8
83f5100
c6eb414
4af63c8
5de58e3
65983a9
ea6f6c4
d85e13f
ace79eb
1bc60b2
243a6d9
aac60a6
f9c1ff0
1864238
f567c83
9bfa8d1
18fe93b
c5890e5
274c3d0
2f6de7e
40f8265
84d5335
b8dbb6a
b438cc5
89559b2
fbedbb5
14d2c24
0412ece
2e4a5b8
2a06ecd
f798a92
278dc3b
0779958
996ebab
291192a
8da8df5
126aff9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import { ICancellationToken } from "@microsoft/powerquery-parser"; | ||
|
|
||
| /** | ||
| * Sequential processing with cancellation support - often better than Promise.all | ||
| * for cancellation scenarios because we can stop between each operation. | ||
| */ | ||
| export async function processSequentiallyWithCancellation<T, R>( | ||
| items: T[], | ||
| processor: (item: T) => Promise<R>, | ||
| cancellationToken?: ICancellationToken, | ||
| ): Promise<R[]> { | ||
| const results: R[] = []; | ||
|
|
||
| for (const item of items) { | ||
| cancellationToken?.throwIfCancelled(); | ||
| // eslint-disable-next-line no-await-in-loop | ||
| const result: R = await processor(item); | ||
| results.push(result); | ||
| } | ||
|
|
||
| return results; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,13 +1,15 @@ | ||
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT license. | ||
|
|
||
| import { CommonError, Result, ResultUtils } from "@microsoft/powerquery-parser"; | ||
| import { NodeIdMap, ParseError, ParseState } from "@microsoft/powerquery-parser/lib/powerquery-parser/parser"; | ||
| import { Diagnostic } from "vscode-languageserver-types"; | ||
| import { TextDocument } from "vscode-languageserver-textdocument"; | ||
| import { Trace } from "@microsoft/powerquery-parser/lib/powerquery-parser/common/trace"; | ||
|
|
||
| import * as PromiseUtils from "../promiseUtils"; | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: can we remove whitespace? |
||
| import { Analysis, AnalysisSettings, AnalysisUtils } from "../analysis"; | ||
| import { CommonError, Result, ResultUtils } from "@microsoft/powerquery-parser"; | ||
| import { TypeCache } from "../inspection"; | ||
| import { validateDuplicateIdentifiers } from "./validateDuplicateIdentifiers"; | ||
| import { validateFunctionExpression } from "./validateFunctionExpression"; | ||
|
|
@@ -36,58 +38,69 @@ export function validate( | |
| }; | ||
|
|
||
| const analysis: Analysis = AnalysisUtils.analysis(textDocument, analysisSettings); | ||
|
|
||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: remove whitespace? |
||
| const parseState: ParseState | undefined = ResultUtils.assertOk(await analysis.getParseState()); | ||
| validationSettings.cancellationToken?.throwIfCancelled(); | ||
|
|
||
| const parseError: ParseError.ParseError | undefined = ResultUtils.assertOk(await analysis.getParseError()); | ||
| validationSettings.cancellationToken?.throwIfCancelled(); | ||
|
|
||
| if (parseState === undefined) { | ||
| trace.exit(); | ||
|
|
||
| return undefined; | ||
| } | ||
|
|
||
| let functionExpressionDiagnostics: Diagnostic[]; | ||
| let invokeExpressionDiagnostics: Diagnostic[]; | ||
| let unknownIdentifiersDiagnostics: Diagnostic[]; | ||
|
|
||
| const nodeIdMapCollection: NodeIdMap.Collection = parseState.contextState.nodeIdMapCollection; | ||
| const typeCache: TypeCache = analysis.getTypeCache(); | ||
|
|
||
| if (validationSettings.checkInvokeExpressions && nodeIdMapCollection) { | ||
| functionExpressionDiagnostics = validateFunctionExpression(validationSettings, nodeIdMapCollection); | ||
| // Define validation operations to run sequentially | ||
| const validationOperations: (() => Promise<Diagnostic[]>)[] = [ | ||
| // Parse validation (if there are parse errors) | ||
| async (): Promise<Diagnostic[]> => await validateParse(parseError, updatedSettings), | ||
| ]; | ||
|
|
||
| invokeExpressionDiagnostics = await validateInvokeExpression( | ||
| validationSettings, | ||
| nodeIdMapCollection, | ||
| typeCache, | ||
| // Add conditional validations based on settings | ||
| if (validationSettings.checkForDuplicateIdentifiers && nodeIdMapCollection) { | ||
| validationOperations.push( | ||
| async (): Promise<Diagnostic[]> => | ||
| await validateDuplicateIdentifiers( | ||
| textDocument, | ||
| nodeIdMapCollection, | ||
| updatedSettings, | ||
| validationSettings.cancellationToken, | ||
| ), | ||
| ); | ||
| } | ||
|
|
||
| if (validationSettings.checkInvokeExpressions && nodeIdMapCollection) { | ||
| validationOperations.push( | ||
| async (): Promise<Diagnostic[]> => | ||
| await validateFunctionExpression(validationSettings, nodeIdMapCollection), | ||
| async (): Promise<Diagnostic[]> => | ||
| await validateInvokeExpression(validationSettings, nodeIdMapCollection, typeCache), | ||
| ); | ||
| } else { | ||
| functionExpressionDiagnostics = []; | ||
| invokeExpressionDiagnostics = []; | ||
| } | ||
|
|
||
| if (validationSettings.checkUnknownIdentifiers && nodeIdMapCollection) { | ||
| unknownIdentifiersDiagnostics = await validateUnknownIdentifiers( | ||
| validationSettings, | ||
| nodeIdMapCollection, | ||
| typeCache, | ||
| validationOperations.push( | ||
| async (): Promise<Diagnostic[]> => | ||
| await validateUnknownIdentifiers(validationSettings, nodeIdMapCollection, typeCache), | ||
| ); | ||
| } else { | ||
| unknownIdentifiersDiagnostics = []; | ||
| } | ||
|
|
||
| // Execute all validation operations sequentially with cancellation support | ||
| const allDiagnostics: Diagnostic[][] = await PromiseUtils.processSequentiallyWithCancellation( | ||
| validationOperations, | ||
| (operation: () => Promise<Diagnostic[]>) => operation(), | ||
| validationSettings.cancellationToken, | ||
| ); | ||
|
|
||
| // Flatten all diagnostics into a single array | ||
| const flattenedDiagnostics: Diagnostic[] = allDiagnostics.flat(); | ||
|
|
||
| const result: ValidateOk = { | ||
| diagnostics: [ | ||
| ...validateDuplicateIdentifiers( | ||
| textDocument, | ||
| nodeIdMapCollection, | ||
| updatedSettings, | ||
| validationSettings.cancellationToken, | ||
| ), | ||
| ...(await validateParse(parseError, updatedSettings)), | ||
| ...functionExpressionDiagnostics, | ||
| ...invokeExpressionDiagnostics, | ||
| ...unknownIdentifiersDiagnostics, | ||
| ], | ||
| diagnostics: flattenedDiagnostics, | ||
| hasSyntaxError: parseError !== undefined, | ||
| }; | ||
|
|
||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.