-
Notifications
You must be signed in to change notification settings - Fork 75
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
introduced dedicated interface PrecomputedScopes, updated implementations of DefaultScopeComputation
and DefaultScopeProvider
#1788
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,11 +56,11 @@ export class DefaultScopeProvider implements ScopeProvider { | |
if (precomputed) { | ||
let currentNode: AstNode | undefined = context.container; | ||
do { | ||
const allDescriptions = precomputed.get(currentNode); | ||
if (allDescriptions.length > 0) { | ||
scopes.push(stream(allDescriptions).filter( | ||
desc => this.reflection.isSubtype(desc.type, referenceType))); | ||
} | ||
scopes.push( | ||
precomputed.getStream(currentNode).filter( | ||
desc => this.reflection.isSubtype(desc.type, referenceType) | ||
) | ||
); | ||
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. This creates empty scopes, which was avoided in the previous version. The change might have a performance impact for larger documents. I suggest keeping a check for an empty stream. |
||
currentNode = currentNode.$container; | ||
} while (currentNode); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,6 @@ import type { ParseResult, ParserOptions } from '../parser/langium-parser.js'; | |
import type { ServiceRegistry } from '../service-registry.js'; | ||
import type { LangiumSharedCoreServices } from '../services.js'; | ||
import type { AstNode, AstNodeDescription, Mutable, Reference } from '../syntax-tree.js'; | ||
import type { MultiMap } from '../utils/collections.js'; | ||
import type { Stream } from '../utils/stream.js'; | ||
import { TextDocument } from './documents.js'; | ||
import { CancellationToken } from '../utils/cancellation.js'; | ||
|
@@ -95,10 +94,15 @@ export enum DocumentState { | |
} | ||
|
||
/** | ||
* Result of the scope precomputation phase (`ScopeComputation` service). | ||
* Result of the scope pre-computation phase (`ScopeComputation` service). | ||
* It maps every AST node to the set of symbols that are visible in the subtree of that node. | ||
*/ | ||
export type PrecomputedScopes = MultiMap<AstNode, AstNodeDescription> | ||
export interface PrecomputedScopes { | ||
add(key: AstNode, value: AstNodeDescription): void | ||
addAll(key: AstNode, values: AstNodeDescription[]): void | ||
get(key: AstNode): readonly AstNodeDescription[] | ||
getStream(key: AstNode): Stream<AstNodeDescription> | ||
} | ||
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. This is a breaking change; we should defer the PR to v4.0.0. |
||
|
||
export interface DocumentSegment { | ||
readonly range: Range | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion: I don't think that the "default implementation" of
PrecomputedScopes
should be exclusively available as a method inside of the default implementation ofScopeComputation
. Instead, I would much rather have aMultiMapPrecomputedScopes
class, that can be used by adopters that don't want to overrideDefaultScopeComputation
.