From 21ef0b6efe9cd1102d847cf33ff14c9dc6ff3d87 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sat, 23 Nov 2024 20:43:03 +0100 Subject: [PATCH 1/8] Document highlights feature --- .../lib/src/language_server.dart | 23 +++++++ .../configuration/language_configuration.dart | 43 ++++--------- .../document_highlights_feature.dart | 60 +++++++++++++++++++ .../find_references_feature.dart | 13 +--- .../lib/src/language_services.dart | 8 +++ .../lib/src/utils/sass_lsp_utils.dart | 11 ++++ 6 files changed, 118 insertions(+), 40 deletions(-) create mode 100644 pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart diff --git a/pkgs/sass_language_server/lib/src/language_server.dart b/pkgs/sass_language_server/lib/src/language_server.dart index 2138d85..19e574b 100644 --- a/pkgs/sass_language_server/lib/src/language_server.dart +++ b/pkgs/sass_language_server/lib/src/language_server.dart @@ -174,6 +174,7 @@ class LanguageServer { var serverCapabilities = ServerCapabilities( definitionProvider: Either2.t1(true), + documentHighlightProvider: Either2.t1(true), documentLinkProvider: DocumentLinkOptions(resolveProvider: false), documentSymbolProvider: Either2.t1(true), referencesProvider: Either2.t1(true), @@ -265,6 +266,28 @@ class LanguageServer { } }); + _connection.onDocumentHighlight((params) async { + try { + var document = _documents.get(params.textDocument.uri); + if (document == null) return []; + + var configuration = _getLanguageConfiguration(document); + if (configuration.highlights.enabled) { + if (initialScan != null) { + await initialScan; + } + + var result = _ls.findDocumentHighlights(document, params.position); + return result; + } else { + return []; + } + } on Exception catch (e) { + _log.debug(e.toString()); + return []; + } + }); + _connection.onDocumentLinks((params) async { try { var document = _documents.get(params.textDocument.uri); diff --git a/pkgs/sass_language_services/lib/src/configuration/language_configuration.dart b/pkgs/sass_language_services/lib/src/configuration/language_configuration.dart index 4693737..3e7cd73 100644 --- a/pkgs/sass_language_services/lib/src/configuration/language_configuration.dart +++ b/pkgs/sass_language_services/lib/src/configuration/language_configuration.dart @@ -4,43 +4,26 @@ class FeatureConfiguration { FeatureConfiguration({required this.enabled}); } -class DefinitionConfiguration extends FeatureConfiguration { - DefinitionConfiguration({required super.enabled}); -} - -class DocumentSymbolsConfiguration extends FeatureConfiguration { - DocumentSymbolsConfiguration({required super.enabled}); -} - -class DocumentLinksConfiguration extends FeatureConfiguration { - DocumentLinksConfiguration({required super.enabled}); -} - -class ReferencesConfiguration extends FeatureConfiguration { - ReferencesConfiguration({required super.enabled}); -} - -class WorkspaceSymbolsConfiguration extends FeatureConfiguration { - WorkspaceSymbolsConfiguration({required super.enabled}); -} - class LanguageConfiguration { - late final DefinitionConfiguration definition; - late final DocumentSymbolsConfiguration documentSymbols; - late final DocumentLinksConfiguration documentLinks; - late final ReferencesConfiguration references; - late final WorkspaceSymbolsConfiguration workspaceSymbols; + late final FeatureConfiguration definition; + late final FeatureConfiguration highlights; + late final FeatureConfiguration documentSymbols; + late final FeatureConfiguration documentLinks; + late final FeatureConfiguration references; + late final FeatureConfiguration workspaceSymbols; LanguageConfiguration.from(dynamic config) { - definition = DefinitionConfiguration( + definition = FeatureConfiguration( enabled: config?['definition']?['enabled'] as bool? ?? true); - documentSymbols = DocumentSymbolsConfiguration( + documentSymbols = FeatureConfiguration( enabled: config?['documentSymbols']?['enabled'] as bool? ?? true); - documentLinks = DocumentLinksConfiguration( + documentLinks = FeatureConfiguration( enabled: config?['documentLinks']?['enabled'] as bool? ?? true); - references = ReferencesConfiguration( + highlights = FeatureConfiguration( + enabled: config?['highlights']?['enabled'] as bool? ?? true); + references = FeatureConfiguration( enabled: config?['references']?['enabled'] as bool? ?? true); - workspaceSymbols = WorkspaceSymbolsConfiguration( + workspaceSymbols = FeatureConfiguration( enabled: config?['workspaceSymbols']?['enabled'] as bool? ?? true); } } diff --git a/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart b/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart new file mode 100644 index 0000000..0a6294e --- /dev/null +++ b/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart @@ -0,0 +1,60 @@ +import 'package:lsp_server/lsp_server.dart' as lsp; +import 'package:sass_api/sass_api.dart' as sass; +import 'package:sass_language_services/sass_language_services.dart'; +import 'package:sass_language_services/src/features/find_references/find_references_visitor.dart'; +import 'package:sass_language_services/src/utils/sass_lsp_utils.dart'; + +import '../go_to_definition/scope_visitor.dart'; +import '../go_to_definition/scoped_symbols.dart'; +import '../language_feature.dart'; +import '../node_at_offset_visitor.dart'; + +class DocumentHighlightsFeature extends LanguageFeature { + DocumentHighlightsFeature({required super.ls}); + + List findDocumentHighlights( + TextDocument document, lsp.Position position) { + var stylesheet = ls.parseStylesheet(document); + // Find the node whose definition we're looking for. + var offset = document.offsetAt(position); + var visitor = NodeAtOffsetVisitor(offset); + var node = stylesheet.accept(visitor); + if (node == null || node is sass.Stylesheet) { + return []; + } + + var name = getNodeName(node); + if (name == null) { + return []; + } + var kind = getNodeReferenceKind(node); + + var symbols = ScopedSymbols( + stylesheet, + document.languageId == 'sass' ? Dialect.indented : Dialect.scss, + ); + var symbol = symbols.findSymbolFromNode(node); + + var result = []; + var references = FindReferencesVisitor(document, name); + for (var reference in references.candidates) { + if (symbol != null) { + if (symbol.referenceKind == reference.kind && + symbol.name == reference.name && + isSameRange(symbol.range, reference.location.range)) { + result.add( + lsp.DocumentHighlight(range: reference.location.range), + ); + } + } else if (kind != null && + reference.kind == kind && + reference.name == name) { + result.add( + lsp.DocumentHighlight(range: reference.location.range), + ); + } + } + + return result; + } +} diff --git a/pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart b/pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart index 91834bc..5f29f1c 100644 --- a/pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart @@ -4,6 +4,7 @@ import 'package:sass_language_services/src/features/find_references/find_referen import 'package:sass_language_services/src/features/go_to_definition/go_to_definition_feature.dart'; import '../../sass/sass_data.dart'; +import '../../utils/sass_lsp_utils.dart'; import '../go_to_definition/definition.dart'; import 'reference.dart'; @@ -89,7 +90,7 @@ class FindReferencesFeature extends GoToDefinitionFeature { continue; } - var candidateIsDefinition = _isSameLocation( + var candidateIsDefinition = isSameLocation( candidate.location, definition.location!, ); @@ -109,7 +110,7 @@ class FindReferencesFeature extends GoToDefinitionFeature { if (candidateDefinition != null && candidateDefinition.location != null) { - if (_isSameLocation( + if (isSameLocation( candidateDefinition.location!, definition.location!, )) { @@ -125,12 +126,4 @@ class FindReferencesFeature extends GoToDefinitionFeature { return (definition: definition, references: references); } - - bool _isSameLocation(lsp.Location a, lsp.Location b) { - return a.uri.toString() == b.uri.toString() && - a.range.start.line == b.range.start.line && - a.range.start.character == b.range.start.character && - a.range.end.line == b.range.end.line && - a.range.end.character == b.range.end.character; - } } diff --git a/pkgs/sass_language_services/lib/src/language_services.dart b/pkgs/sass_language_services/lib/src/language_services.dart index 40a51d4..085d206 100644 --- a/pkgs/sass_language_services/lib/src/language_services.dart +++ b/pkgs/sass_language_services/lib/src/language_services.dart @@ -1,6 +1,7 @@ import 'package:lsp_server/lsp_server.dart' as lsp; import 'package:sass_api/sass_api.dart' as sass; import 'package:sass_language_services/sass_language_services.dart'; +import 'package:sass_language_services/src/features/document_highlights/document_highlights_feature.dart'; import 'package:sass_language_services/src/features/find_references/find_references_feature.dart'; import 'package:sass_language_services/src/features/go_to_definition/go_to_definition_feature.dart'; @@ -17,6 +18,7 @@ class LanguageServices { LanguageServerConfiguration configuration = LanguageServerConfiguration.create(null); + late final DocumentHighlightsFeature _documentHighlights; late final DocumentLinksFeature _documentLinks; late final DocumentSymbolsFeature _documentSymbols; late final GoToDefinitionFeature _goToDefinition; @@ -27,6 +29,7 @@ class LanguageServices { required this.clientCapabilities, required this.fs, }) : cache = LanguageServicesCache() { + _documentHighlights = DocumentHighlightsFeature(ls: this); _documentLinks = DocumentLinksFeature(ls: this); _documentSymbols = DocumentSymbolsFeature(ls: this); _goToDefinition = GoToDefinitionFeature(ls: this); @@ -38,6 +41,11 @@ class LanguageServices { this.configuration = configuration; } + List findDocumentHighlights( + TextDocument document, lsp.Position position) { + return _documentHighlights.findDocumentHighlights(document, position); + } + Future> findDocumentLinks( TextDocument document) { return _documentLinks.findDocumentLinks(document); diff --git a/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart b/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart index 43975df..bef038f 100644 --- a/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart +++ b/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart @@ -51,3 +51,14 @@ bool isInRange({required lsp.Position position, required lsp.Range range}) { range.end.line >= position.line && range.end.character >= position.character; } + +bool isSameLocation(lsp.Location a, lsp.Location b) { + return a.uri.toString() == b.uri.toString() && isSameRange(a.range, b.range); +} + +bool isSameRange(lsp.Range a, lsp.Range b) { + return a.start.line == b.start.line && + a.start.character == b.start.character && + a.end.line == b.end.line && + a.end.character == b.end.character; +} From 5cd42b502e0553717fb815ae55f4f5caceb71603 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sat, 23 Nov 2024 20:49:05 +0100 Subject: [PATCH 2/8] Default turn on features in the test extension --- extension/package.json | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/extension/package.json b/extension/package.json index 5b362a3..e8d974a 100644 --- a/extension/package.json +++ b/extension/package.json @@ -224,14 +224,14 @@ "order": 60, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable folding ranges." }, "sass.scss.highlights.enabled": { "order": 70, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable highlights." }, "sass.scss.hover.enabled": { @@ -245,7 +245,7 @@ "order": 81, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Show property and value documentation in CSS hovers." }, "sass.scss.hover.references": { @@ -280,7 +280,7 @@ "order": 120, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable selection ranges." }, "sass.scss.signatureHelp.enabled": { @@ -471,21 +471,21 @@ "order": 10, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable all code actions." }, "sass.css.colors.enabled": { "order": 20, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable all color decorators." }, "sass.css.completion.enabled": { "order": 30, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable all completions (IntelliSense)." }, "sass.css.completion.triggerPropertyValueCompletion": { @@ -506,14 +506,14 @@ "order": 40, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable Go to Definition." }, "sass.css.diagnostics.enabled": { "order": 50, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable all diagnostics." }, "sass.css.documentSymbols.enabled": { @@ -527,21 +527,21 @@ "order": 60, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable folding ranges." }, "sass.css.highlights.enabled": { "order": 70, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable highlights." }, "sass.css.hover.enabled": { "order": 80, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable all hover information." }, "sass.css.hover.documentation": { @@ -562,42 +562,42 @@ "order": 90, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable the link provider that lets you click an import and open the file." }, "sass.css.references.enabled": { "order": 100, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable Find all references." }, "sass.css.rename.enabled": { "order": 110, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable Rename." }, "sass.css.selectionRanges.enabled": { "order": 120, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable selection ranges." }, "sass.css.signatureHelp.enabled": { "order": 130, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable signature help." }, "sass.css.workspaceSymbols.enabled": { "order": 140, "type": "boolean", "scope": "resource", - "default": false, + "default": true, "description": "Enable or disable workspace symbols." } } From fab73ad856fc565542cbf2f130843714a61ee2ac Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sat, 23 Nov 2024 21:10:11 +0100 Subject: [PATCH 3/8] Cache scoped symbols, not document symbols Designed the cache with the assumption that findDocumentSymbols would be on the hot path for workspace traversal, findReferences etc. Forgot to update once that turned out not to be the case. --- .../document_highlights_feature.dart | 18 +++++++++++----- .../document_symbols_feature.dart | 7 ------- .../go_to_definition_feature.dart | 18 ++++++++++++---- .../go_to_definition/scoped_symbols.dart | 21 ++++++++++++------- .../lib/src/language_services_cache.dart | 8 +++---- 5 files changed, 45 insertions(+), 27 deletions(-) diff --git a/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart b/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart index 0a6294e..f2ee3c3 100644 --- a/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart @@ -29,14 +29,22 @@ class DocumentHighlightsFeature extends LanguageFeature { } var kind = getNodeReferenceKind(node); - var symbols = ScopedSymbols( - stylesheet, - document.languageId == 'sass' ? Dialect.indented : Dialect.scss, - ); + var symbols = ls.cache.getDocumentSymbols(document) ?? + ScopedSymbols( + stylesheet, + document.languageId == 'sass' ? Dialect.indented : Dialect.scss, + ); + ls.cache.setDocumentSymbols(document, symbols); + var symbol = symbols.findSymbolFromNode(node); var result = []; - var references = FindReferencesVisitor(document, name); + var references = FindReferencesVisitor( + document, + name, + includeDeclaration: true, + ); + for (var reference in references.candidates) { if (symbol != null) { if (symbol.referenceKind == reference.kind && diff --git a/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_feature.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_feature.dart index b2ab6a9..0c2966d 100644 --- a/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_feature.dart @@ -7,18 +7,11 @@ class DocumentSymbolsFeature extends LanguageFeature { DocumentSymbolsFeature({required super.ls}); List findDocumentSymbols(TextDocument document) { - final cached = ls.cache.getDocumentSymbols(document); - if (cached != null) { - return cached; - } - var stylesheet = ls.parseStylesheet(document); var symbolsVisitor = DocumentSymbolsVisitor(); stylesheet.accept(symbolsVisitor); - ls.cache.setDocumentSymbols(document, symbolsVisitor.symbols); - return symbolsVisitor.symbols; } } diff --git a/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart b/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart index 6869ed4..b0a2140 100644 --- a/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart @@ -60,8 +60,12 @@ class GoToDefinitionFeature extends LanguageFeature { // Look for the symbol in the current document. // It may be a scoped symbol. - var symbols = ScopedSymbols(stylesheet, - document.languageId == 'sass' ? Dialect.indented : Dialect.scss); + var symbols = ls.cache.getDocumentSymbols(document) ?? + ScopedSymbols( + stylesheet, + document.languageId == 'sass' ? Dialect.indented : Dialect.scss, + ); + ls.cache.setDocumentSymbols(document, symbols); var symbol = symbols.findSymbolFromNode(node); if (symbol != null) { // Found the definition in the same document. @@ -128,8 +132,14 @@ class GoToDefinitionFeature extends LanguageFeature { : name!; var stylesheet = ls.parseStylesheet(document); - var symbols = ScopedSymbols(stylesheet, - document.languageId == 'sass' ? Dialect.indented : Dialect.scss); + + var symbols = ls.cache.getDocumentSymbols(document) ?? + ScopedSymbols( + stylesheet, + document.languageId == 'sass' ? Dialect.indented : Dialect.scss, + ); + ls.cache.setDocumentSymbols(document, symbols); + var symbol = symbols.globalScope.getSymbol( name: unprefixedName, referenceKind: kind, diff --git a/pkgs/sass_language_services/lib/src/features/go_to_definition/scoped_symbols.dart b/pkgs/sass_language_services/lib/src/features/go_to_definition/scoped_symbols.dart index 0a5cb9a..f8dd92d 100644 --- a/pkgs/sass_language_services/lib/src/features/go_to_definition/scoped_symbols.dart +++ b/pkgs/sass_language_services/lib/src/features/go_to_definition/scoped_symbols.dart @@ -11,13 +11,14 @@ ReferenceKind? getNodeReferenceKind(sass.AstNode node) { return ReferenceKind.variable; } else if (node is sass.Declaration) { var isCustomProperty = - node.name.isPlain && node.name.asPlain!.startsWith("--"); + node.name.isPlain && node.name.asPlain!.startsWith('--'); if (isCustomProperty) { return ReferenceKind.customProperty; } } else if (node is sass.StringExpression) { - var isCustomProperty = - node.text.isPlain && node.text.asPlain!.startsWith("--"); + var isCustomProperty = node.text.isPlain && + (node.text.asPlain!.startsWith('--') || + node.text.asPlain!.startsWith('var(--')); if (isCustomProperty) { return ReferenceKind.customProperty; } @@ -60,15 +61,21 @@ String? getNodeName(sass.AstNode node) { return node.name; } else if (node is sass.Declaration) { var isCustomProperty = - node.name.isPlain && node.name.asPlain!.startsWith("--"); + node.name.isPlain && node.name.asPlain!.startsWith('--'); if (isCustomProperty) { return node.name.asPlain; } } else if (node is sass.StringExpression) { - var isCustomProperty = - node.text.isPlain && node.text.asPlain!.startsWith("--"); + var isCustomProperty = node.text.isPlain && + (node.text.asPlain!.startsWith('--') || + node.text.asPlain!.startsWith('var(--')); if (isCustomProperty) { - return node.text.asPlain; + var name = node.text.asPlain!; + if (name.startsWith('var(')) { + name = name.replaceAll('var(', ''); + name = name.replaceAll(')', ''); + } + return name; } } else if (node is sass.AtRule) { var name = node.name; diff --git a/pkgs/sass_language_services/lib/src/language_services_cache.dart b/pkgs/sass_language_services/lib/src/language_services_cache.dart index 2b2a3c2..8b0d7aa 100644 --- a/pkgs/sass_language_services/lib/src/language_services_cache.dart +++ b/pkgs/sass_language_services/lib/src/language_services_cache.dart @@ -1,11 +1,12 @@ import 'package:sass_api/sass_api.dart' as sass; import 'package:sass_language_services/sass_language_services.dart'; +import 'package:sass_language_services/src/features/go_to_definition/scoped_symbols.dart'; class CacheEntry { TextDocument document; sass.Stylesheet stylesheet; List? links; - List? symbols; + ScopedSymbols? symbols; CacheEntry({ required this.document, @@ -53,7 +54,7 @@ class LanguageServicesCache { return _cache[document.uri.toString()]?.links; } - List? getDocumentSymbols(TextDocument document) { + ScopedSymbols? getDocumentSymbols(TextDocument document) { return _cache[document.uri.toString()]?.symbols; } @@ -62,8 +63,7 @@ class LanguageServicesCache { _cache[document.uri.toString()]?.links = links; } - void setDocumentSymbols( - TextDocument document, List symbols) { + void setDocumentSymbols(TextDocument document, ScopedSymbols symbols) { _cache[document.uri.toString()]?.symbols = symbols; } From baaa3113c70e8c8130c4f027c8a6b5cdeeb437b1 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sat, 23 Nov 2024 21:53:10 +0100 Subject: [PATCH 4/8] Rewrite scope lookup for performance Linear search didn't scale well, turns out. --- .../src/features/go_to_definition/scope.dart | 51 +++++++++++++++---- .../features/go_to_definition/scope_test.dart | 4 +- 2 files changed, 44 insertions(+), 11 deletions(-) diff --git a/pkgs/sass_language_services/lib/src/features/go_to_definition/scope.dart b/pkgs/sass_language_services/lib/src/features/go_to_definition/scope.dart index 6024470..604a206 100644 --- a/pkgs/sass_language_services/lib/src/features/go_to_definition/scope.dart +++ b/pkgs/sass_language_services/lib/src/features/go_to_definition/scope.dart @@ -21,26 +21,59 @@ class Scope { } Scope? findScope({required int offset, int length = 0}) { - if ((this.offset <= offset && + var scopeContainsOffset = (this.offset <= offset && this.offset + this.length > offset + length) || - (this.offset == offset && this.length == length)) { + (this.offset == offset && this.length == length); + + if (scopeContainsOffset) { return findInScope(offset: offset, length: length); } + return null; } + /// Assumes a sorted [list], where [matcher] would return false + /// for all elements before it returns true. This lets us do a bisect + /// to quickly find the first element, as opposed to a linear firstWhere. + int _first(List list, bool Function(T item) matcher) { + if (list.isEmpty) { + return 0; + } + + var low = 0; + var high = list.length; + + while (low < high) { + var half = ((low + high) / 2).floor(); + if (matcher(list[half])) { + high = half; + } else { + low = half + 1; + } + } + + return low; + } + Scope findInScope({required int offset, int length = 0}) { - var scopeAtOffset = children.firstWhere( - (scope) => - scope.offset <= offset && - scope.offset + scope.length >= offset + length, - orElse: () => this); + var end = offset + length; + var scopeIndex = _first( + children, + (scope) => scope.offset > end, + ); - if (scopeAtOffset == this) { + if (scopeIndex == 0) { return this; } - return scopeAtOffset.findInScope(offset: offset, length: length); + var candidate = children.elementAt(scopeIndex - 1); + var containsOffset = candidate.offset <= offset && + candidate.offset + candidate.length >= offset + length; + if (containsOffset) { + return candidate.findInScope(offset: offset, length: length); + } + + return this; } void addSymbol(StylesheetDocumentSymbol symbol) { diff --git a/pkgs/sass_language_services/test/features/go_to_definition/scope_test.dart b/pkgs/sass_language_services/test/features/go_to_definition/scope_test.dart index b76b994..3b2a390 100644 --- a/pkgs/sass_language_services/test/features/go_to_definition/scope_test.dart +++ b/pkgs/sass_language_services/test/features/go_to_definition/scope_test.dart @@ -42,8 +42,8 @@ void main() { expect(global.findScope(offset: 21), equals(global)); expect(global.findScope(offset: 10), equals(first)); - expect(global.findScope(offset: 15), equals(first)); - expect(global.findScope(offset: 16), equals(second)); + expect(global.findScope(offset: 14), equals(first)); + expect(global.findScope(offset: 15), equals(second)); expect(global.findScope(offset: 20), equals(second)); }); }); From 285bb54ac43adebda581cbcfb8acae66e64ef7d4 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 24 Nov 2024 08:36:44 +0100 Subject: [PATCH 5/8] Have findInWorkspace return visited documents In case the feature can use it to avoid duplicate work --- .../go_to_definition_feature.dart | 19 +++++++--- .../lib/src/features/language_feature.dart | 36 +++++++++++-------- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart b/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart index b0a2140..d30569c 100644 --- a/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart @@ -109,7 +109,7 @@ class GoToDefinitionFeature extends LanguageFeature { } } - var definition = + var result = await findInWorkspace<(StylesheetDocumentSymbol, lsp.Location)>( lazy: true, initialDocument: initialDocument, @@ -158,6 +158,9 @@ class GoToDefinitionFeature extends LanguageFeature { }, ); + var definition = result.result; + var visited = result.visited; + if (definition != null && definition.isNotEmpty) { var symbol = definition.first.$1; var location = definition.first.$2; @@ -168,11 +171,19 @@ class GoToDefinitionFeature extends LanguageFeature { ); } - // Fall back to "@import-style" lookup on the whole workspace. + // Fall back to "@import-style" lookup on the rest of the workspace. for (var document in ls.cache.getDocuments()) { + if (visited.contains(document.uri.toString())) { + continue; + } + var stylesheet = ls.parseStylesheet(document); - var symbols = ScopedSymbols(stylesheet, - document.languageId == 'sass' ? Dialect.indented : Dialect.scss); + var symbols = ls.cache.getDocumentSymbols(document) ?? + ScopedSymbols( + stylesheet, + document.languageId == 'sass' ? Dialect.indented : Dialect.scss, + ); + ls.cache.setDocumentSymbols(document, symbols); for (var kind in kinds) { var symbol = symbols.globalScope.getSymbol( diff --git a/pkgs/sass_language_services/lib/src/features/language_feature.dart b/pkgs/sass_language_services/lib/src/features/language_feature.dart index 9e3274a..1494a59 100644 --- a/pkgs/sass_language_services/lib/src/features/language_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/language_feature.dart @@ -3,6 +3,13 @@ import 'dart:math'; import '../../sass_language_services.dart'; import '../utils/uri_utils.dart'; +class WorkspaceResult { + final List? result; + final Set visited; + + WorkspaceResult(this.result, this.visited); +} + abstract class LanguageFeature { late final LanguageServices ls; @@ -11,7 +18,7 @@ abstract class LanguageFeature { /// Helper to do some kind of lookup for the import tree of [initialDocument]. /// /// The [callback] is called for each document in the import tree. Documents will only get visited once. - Future?> findInWorkspace( + Future> findInWorkspace( {required Future?> Function({ required TextDocument document, required String prefix, @@ -37,7 +44,7 @@ abstract class LanguageFeature { ); } - Future?> _findInWorkspace( + Future> _findInWorkspace( {required Future?> Function({ required TextDocument document, required String prefix, @@ -57,19 +64,20 @@ abstract class LanguageFeature { bool lazy = false, int depth = 0}) async { if (visited.contains(currentDocument.uri.toString())) { - return Future.value([]); + return Future.value(WorkspaceResult([], visited)); } var result = await callback( - document: currentDocument, - prefix: accumulatedPrefix, - hiddenMixinsAndFunctions: hiddenMixinsAndFunctions, - hiddenVariables: hiddenVariables, - shownMixinsAndFunctions: shownMixinsAndFunctions, - shownVariables: shownVariables); + document: currentDocument, + prefix: accumulatedPrefix, + hiddenMixinsAndFunctions: hiddenMixinsAndFunctions, + hiddenVariables: hiddenVariables, + shownMixinsAndFunctions: shownMixinsAndFunctions, + shownVariables: shownVariables, + ); if (lazy && result != null) { - return result; + return WorkspaceResult(result, visited); } visited.add(currentDocument.uri.toString()); @@ -90,7 +98,7 @@ abstract class LanguageFeature { }); if (links.isEmpty) { - return null; + return WorkspaceResult([], visited); } var linksResult = []; @@ -143,12 +151,12 @@ abstract class LanguageFeature { depth: depth + 1, ); - if (linkResult != null) { - linksResult.addAll(linkResult); + if (linkResult.result != null) { + linksResult.addAll(linkResult.result!); } } - return linksResult; + return WorkspaceResult(linksResult, visited); } Future getTextDocument(Uri uri) async { From 8f765976b6ac639b3ef4571e82727bf32539bf5f Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 24 Nov 2024 09:16:02 +0100 Subject: [PATCH 6/8] Refactor Node at offset visitor Trying to reduce time spent in the visitor --- .../document_highlights_feature.dart | 4 +- .../go_to_definition_feature.dart | 3 +- .../src/features/node_at_offset_visitor.dart | 123 ++++++++++-------- 3 files changed, 70 insertions(+), 60 deletions(-) diff --git a/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart b/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart index f2ee3c3..dd59eae 100644 --- a/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart @@ -15,10 +15,10 @@ class DocumentHighlightsFeature extends LanguageFeature { List findDocumentHighlights( TextDocument document, lsp.Position position) { var stylesheet = ls.parseStylesheet(document); + // Find the node whose definition we're looking for. var offset = document.offsetAt(position); - var visitor = NodeAtOffsetVisitor(offset); - var node = stylesheet.accept(visitor); + var node = getNodeAtOffset(stylesheet, offset); if (node == null || node is sass.Stylesheet) { return []; } diff --git a/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart b/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart index d30569c..f6dafcf 100644 --- a/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart @@ -29,8 +29,7 @@ class GoToDefinitionFeature extends LanguageFeature { // Find the node whose definition we're looking for. var offset = document.offsetAt(position); - var visitor = NodeAtOffsetVisitor(offset); - var node = stylesheet.accept(visitor); + var node = getNodeAtOffset(stylesheet, offset); if (node == null) { return null; } diff --git a/pkgs/sass_language_services/lib/src/features/node_at_offset_visitor.dart b/pkgs/sass_language_services/lib/src/features/node_at_offset_visitor.dart index ab72e55..01a40ac 100644 --- a/pkgs/sass_language_services/lib/src/features/node_at_offset_visitor.dart +++ b/pkgs/sass_language_services/lib/src/features/node_at_offset_visitor.dart @@ -1,29 +1,40 @@ import 'package:sass_api/sass_api.dart' as sass; +sass.AstNode? getNodeAtOffset(sass.ParentStatement node, int offset) { + if (node.span.start.offset > offset || offset > node.span.end.offset) { + return null; + } + var visitor = NodeAtOffsetVisitor(offset); + var result = node.accept(visitor); + // The visitor might have reached the end of the syntax tree, + // in which case result is null. We still might have a candidate. + return result ?? visitor.candidate; +} + class NodeAtOffsetVisitor with sass.StatementSearchVisitor, sass.AstSearchVisitor { - sass.AstNode? _candidate; + sass.AstNode? candidate; final int _offset; /// Finds the node with the shortest span at [offset]. NodeAtOffsetVisitor(int offset) : _offset = offset; sass.AstNode? _process(sass.AstNode node) { - var spanContainsOffset = + var containsOffset = node.span.start.offset <= _offset && node.span.end.offset >= _offset; - if (spanContainsOffset) { - if (_candidate == null) { - _candidate = node; - } else if (node.span.length <= _candidate!.span.length) { - _candidate = node; + if (containsOffset) { + if (candidate == null) { + candidate = node; + } else if (node.span.length <= candidate!.span.length) { + candidate = node; } } - if (node.span.start.offset > _offset || node is sass.Stylesheet) { - return _candidate; + if (node.span.start.offset > _offset) { + return candidate; } return null; @@ -31,239 +42,239 @@ class NodeAtOffsetVisitor @override sass.AstNode? visitArgumentInvocation(sass.ArgumentInvocation invocation) { - return super.visitArgumentInvocation(invocation) ?? _process(invocation); + return _process(invocation) ?? super.visitArgumentInvocation(invocation); } @override sass.AstNode? visitAtRootRule(sass.AtRootRule node) { - return super.visitAtRootRule(node) ?? _process(node); + return _process(node) ?? super.visitAtRootRule(node); } @override sass.AstNode? visitAtRule(sass.AtRule node) { - return super.visitAtRule(node) ?? _process(node); + return _process(node) ?? super.visitAtRule(node); } @override sass.AstNode? visitBinaryOperationExpression( sass.BinaryOperationExpression node) { - return super.visitBinaryOperationExpression(node) ?? _process(node); + return _process(node) ?? super.visitBinaryOperationExpression(node); } @override sass.AstNode? visitBooleanExpression(sass.BooleanExpression node) { - return super.visitBooleanExpression(node) ?? _process(node); + return _process(node) ?? super.visitBooleanExpression(node); } @override sass.AstNode? visitCallableDeclaration(sass.CallableDeclaration node) { - return super.visitCallableDeclaration(node) ?? _process(node); + return _process(node) ?? super.visitCallableDeclaration(node); } @override sass.AstNode? visitColorExpression(sass.ColorExpression node) { - return super.visitColorExpression(node) ?? _process(node); + return _process(node) ?? super.visitColorExpression(node); } @override sass.AstNode? visitContentBlock(sass.ContentBlock node) { - return super.visitContentBlock(node) ?? _process(node); + return _process(node) ?? super.visitContentBlock(node); } @override sass.AstNode? visitContentRule(sass.ContentRule node) { - return super.visitContentRule(node) ?? _process(node); + return _process(node) ?? super.visitContentRule(node); } @override sass.AstNode? visitDebugRule(sass.DebugRule node) { - return super.visitDebugRule(node) ?? _process(node); + return _process(node) ?? super.visitDebugRule(node); } @override sass.AstNode? visitDeclaration(sass.Declaration node) { - return super.visitDeclaration(node) ?? _process(node); + return _process(node) ?? super.visitDeclaration(node); } @override sass.AstNode? visitEachRule(sass.EachRule node) { - return super.visitEachRule(node) ?? _process(node); + return _process(node) ?? super.visitEachRule(node); } @override sass.AstNode? visitErrorRule(sass.ErrorRule node) { - return super.visitErrorRule(node) ?? _process(node); + return _process(node) ?? super.visitErrorRule(node); } @override sass.AstNode? visitExpression(sass.Expression expression) { - return super.visitExpression(expression) ?? _process(expression); + return _process(expression) ?? super.visitExpression(expression); } @override sass.AstNode? visitExtendRule(sass.ExtendRule node) { - return super.visitExtendRule(node) ?? _process(node); + return _process(node) ?? super.visitExtendRule(node); } @override sass.AstNode? visitForRule(sass.ForRule node) { - return super.visitForRule(node) ?? _process(node); + return _process(node) ?? super.visitForRule(node); } @override sass.AstNode? visitForwardRule(sass.ForwardRule node) { - return super.visitForwardRule(node) ?? _process(node); + return _process(node) ?? super.visitForwardRule(node); } @override sass.AstNode? visitFunctionExpression(sass.FunctionExpression node) { - return super.visitFunctionExpression(node) ?? _process(node); + return _process(node) ?? super.visitFunctionExpression(node); } @override sass.AstNode? visitFunctionRule(sass.FunctionRule node) { - return super.visitFunctionRule(node) ?? _process(node); + return _process(node) ?? super.visitFunctionRule(node); } @override sass.AstNode? visitIfExpression(sass.IfExpression node) { - return super.visitIfExpression(node) ?? _process(node); + return _process(node) ?? super.visitIfExpression(node); } @override sass.AstNode? visitIfRule(sass.IfRule node) { - return super.visitIfRule(node) ?? _process(node); + return _process(node) ?? super.visitIfRule(node); } @override sass.AstNode? visitImportRule(sass.ImportRule node) { - return super.visitImportRule(node) ?? _process(node); + return _process(node) ?? super.visitImportRule(node); } @override sass.AstNode? visitIncludeRule(sass.IncludeRule node) { - return super.visitIncludeRule(node) ?? _process(node); + return _process(node) ?? super.visitIncludeRule(node); } @override sass.AstNode? visitListExpression(sass.ListExpression node) { - return super.visitListExpression(node) ?? _process(node); + return _process(node) ?? super.visitListExpression(node); } @override sass.AstNode? visitLoudComment(sass.LoudComment node) { - return super.visitLoudComment(node) ?? _process(node); + return _process(node) ?? super.visitLoudComment(node); } @override sass.AstNode? visitMapExpression(sass.MapExpression node) { - return super.visitMapExpression(node) ?? _process(node); + return _process(node) ?? super.visitMapExpression(node); } @override sass.AstNode? visitMediaRule(sass.MediaRule node) { - return super.visitMediaRule(node) ?? _process(node); + return _process(node) ?? super.visitMediaRule(node); } @override sass.AstNode? visitMixinRule(sass.MixinRule node) { - return super.visitMixinRule(node) ?? _process(node); + return _process(node) ?? super.visitMixinRule(node); } @override sass.AstNode? visitNullExpression(sass.NullExpression node) { - return super.visitNullExpression(node) ?? _process(node); + return _process(node) ?? super.visitNullExpression(node); } @override sass.AstNode? visitNumberExpression(sass.NumberExpression node) { - return super.visitNumberExpression(node) ?? _process(node); + return _process(node) ?? super.visitNumberExpression(node); } @override sass.AstNode? visitParenthesizedExpression( sass.ParenthesizedExpression node) { - return super.visitParenthesizedExpression(node) ?? _process(node); + return _process(node) ?? super.visitParenthesizedExpression(node); } @override sass.AstNode? visitReturnRule(sass.ReturnRule node) { - return super.visitReturnRule(node) ?? _process(node); + return _process(node) ?? super.visitReturnRule(node); } @override sass.AstNode? visitSelectorExpression(sass.SelectorExpression node) { - return super.visitSelectorExpression(node) ?? _process(node); + return _process(node) ?? super.visitSelectorExpression(node); } @override sass.AstNode? visitSilentComment(sass.SilentComment node) { - return super.visitSilentComment(node) ?? _process(node); + return _process(node) ?? super.visitSilentComment(node); } @override sass.AstNode? visitStringExpression(sass.StringExpression node) { - return super.visitStringExpression(node) ?? _process(node); + return _process(node) ?? super.visitStringExpression(node); } @override sass.AstNode? visitStyleRule(sass.StyleRule node) { - return super.visitStyleRule(node) ?? _process(node); + return _process(node) ?? super.visitStyleRule(node); } @override sass.AstNode? visitStylesheet(sass.Stylesheet node) { - return super.visitStylesheet(node) ?? _process(node); + return _process(node) ?? super.visitStylesheet(node); } @override sass.AstNode? visitSupportsCondition(sass.SupportsCondition condition) { - return super.visitSupportsCondition(condition) ?? _process(condition); + return _process(condition) ?? super.visitSupportsCondition(condition); } @override sass.AstNode? visitSupportsExpression(sass.SupportsExpression node) { - return super.visitSupportsExpression(node) ?? _process(node); + return _process(node) ?? super.visitSupportsExpression(node); } @override sass.AstNode? visitSupportsRule(sass.SupportsRule node) { - return super.visitSupportsRule(node) ?? _process(node); + return _process(node) ?? super.visitSupportsRule(node); } @override sass.AstNode? visitUnaryOperationExpression( sass.UnaryOperationExpression node) { - return super.visitUnaryOperationExpression(node) ?? _process(node); + return _process(node) ?? super.visitUnaryOperationExpression(node); } @override sass.AstNode? visitUseRule(sass.UseRule node) { - return super.visitUseRule(node) ?? _process(node); + return _process(node) ?? super.visitUseRule(node); } @override sass.AstNode? visitValueExpression(sass.ValueExpression node) { - return super.visitValueExpression(node) ?? _process(node); + return _process(node) ?? super.visitValueExpression(node); } @override sass.AstNode? visitVariableDeclaration(sass.VariableDeclaration node) { - return super.visitVariableDeclaration(node) ?? _process(node); + return _process(node) ?? super.visitVariableDeclaration(node); } @override sass.AstNode? visitVariableExpression(sass.VariableExpression node) { - return super.visitVariableExpression(node) ?? _process(node); + return _process(node) ?? super.visitVariableExpression(node); } @override sass.AstNode? visitWarnRule(sass.WarnRule node) { - return super.visitWarnRule(node) ?? _process(node); + return _process(node) ?? super.visitWarnRule(node); } @override sass.AstNode? visitWhileRule(sass.WhileRule node) { - return super.visitWhileRule(node) ?? _process(node); + return _process(node) ?? super.visitWhileRule(node); } } From d2b50dc5e04c4ed63ec0a7205433eacae291ed92 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 24 Nov 2024 09:52:36 +0100 Subject: [PATCH 7/8] Refactor to reuse span as much as possible --- .../document_links/document_link_visitor.dart | 7 +- .../document_symbols_visitor.dart | 61 +++---- .../find_references_visitor.dart | 9 +- .../go_to_definition/scope_visitor.dart | 149 ++++++++++-------- .../src/features/node_at_offset_visitor.dart | 19 ++- .../lib/src/utils/sass_lsp_utils.dart | 23 +-- 6 files changed, 150 insertions(+), 118 deletions(-) diff --git a/pkgs/sass_language_services/lib/src/features/document_links/document_link_visitor.dart b/pkgs/sass_language_services/lib/src/features/document_links/document_link_visitor.dart index 84815be..d89dd1c 100644 --- a/pkgs/sass_language_services/lib/src/features/document_links/document_link_visitor.dart +++ b/pkgs/sass_language_services/lib/src/features/document_links/document_link_visitor.dart @@ -97,17 +97,16 @@ class DocumentLinkVisitor with sass.RecursiveStatementVisitor { } const isSassLink = false; + var urlSpan = staticImport.url.span; unresolvedLinks.add(( StylesheetDocumentLink( type: LinkType.import, target: target, range: lsp.Range( end: lsp.Position( - line: staticImport.url.span.end.line, - character: staticImport.url.span.end.column), + line: urlSpan.end.line, character: urlSpan.end.column), start: lsp.Position( - line: staticImport.url.span.start.line, - character: staticImport.url.span.start.column), + line: urlSpan.start.line, character: urlSpan.start.column), ), ), isSassLink diff --git a/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_visitor.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_visitor.dart index 2224fb8..485f280 100644 --- a/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_visitor.dart +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_visitor.dart @@ -105,30 +105,33 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { } if (node.name.asPlain == 'font-face') { + var nameSpan = node.name.span; _collect( - name: node.name.span.text, + name: nameSpan.text, referenceKind: ReferenceKind.fontFace, symbolRange: toRange(node.span), - nameRange: toRange(node.name.span), + nameRange: toRange(nameSpan), ); } else if (node.name.asPlain!.startsWith('keyframes')) { - var keyframesName = node.span.context.split(' ').elementAtOrNull(1); + var span = node.span; + var nameSpan = node.name.span; + var keyframesName = span.context.split(' ').elementAtOrNull(1); if (keyframesName != null) { var keyframesNameRange = lsp.Range( start: lsp.Position( - line: node.name.span.start.line, - character: node.name.span.end.column + 1, + line: nameSpan.start.line, + character: nameSpan.end.column + 1, ), end: lsp.Position( - line: node.name.span.end.line, - character: node.name.span.end.column + 1 + keyframesName.length, + line: nameSpan.end.line, + character: nameSpan.end.column + 1 + keyframesName.length, ), ); _collect( name: keyframesName, referenceKind: ReferenceKind.keyframe, - symbolRange: toRange(node.span), + symbolRange: toRange(span), nameRange: keyframesNameRange, ); } @@ -141,11 +144,12 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { var isCustomProperty = node.name.isPlain && node.name.asPlain!.startsWith("--"); if (isCustomProperty) { + var nameSpan = node.name.span; _collect( - name: node.name.span.text, + name: nameSpan.text, referenceKind: ReferenceKind.customProperty, symbolRange: toRange(node.span), - nameRange: toRange(node.name.span), + nameRange: toRange(nameSpan), ); } } @@ -172,14 +176,15 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { } // node.query.span includes whitespace, so the range doesn't match node.query.asPlain + var querySpan = node.query.span; var nameRange = lsp.Range( start: lsp.Position( - line: node.query.span.start.line, - character: node.query.span.start.column, + line: querySpan.start.line, + character: querySpan.start.column, ), end: lsp.Position( - line: node.query.span.end.line, - character: node.query.span.start.column + node.query.asPlain!.length, + line: querySpan.end.line, + character: querySpan.start.column + node.query.asPlain!.length, ), ); @@ -222,16 +227,17 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { lsp.Range? symbolRange; for (var component in complexSelector.components) { - var selector = component.selector; + var selectorSpan = component.selector.span; + var span = node.span; if (name == null) { - name = selector.span.text; + name = selectorSpan.text; } else { - name = '$name ${selector.span.text}'; + name = '$name ${selectorSpan.text}'; } if (nameRange == null) { - nameRange = selectorNameRange(node, selector); + nameRange = selectorNameRange(node: span, selector: selectorSpan); // symbolRange: start position of selector's nameRange, end of stylerule (node.span.end). symbolRange = lsp.Range( @@ -240,8 +246,8 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { character: nameRange.start.character, ), end: lsp.Position( - line: node.span.end.line, - character: node.span.end.column, + line: span.end.line, + character: span.end.column, ), ); } else { @@ -249,8 +255,8 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { nameRange = lsp.Range( start: nameRange.start, end: lsp.Position( - line: node.span.start.line + selector.span.end.line, - character: node.span.start.column + selector.span.end.column, + line: span.start.line + selectorSpan.end.line, + character: span.start.column + selectorSpan.end.column, ), ); } @@ -271,20 +277,21 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { @override void visitVariableDeclaration(node) { super.visitVariableDeclaration(node); + var nameSpan = node.nameSpan; _collect( - name: node.nameSpan.text, + name: nameSpan.text, referenceKind: ReferenceKind.variable, docComment: node.comment?.docComment, symbolRange: toRange(node.span), nameRange: lsp.Range( start: lsp.Position( - line: node.nameSpan.start.line, + line: nameSpan.start.line, // the span includes $ - character: node.nameSpan.start.column, + character: nameSpan.start.column, ), end: lsp.Position( - line: node.nameSpan.end.line, - character: node.nameSpan.end.column, + line: nameSpan.end.line, + character: nameSpan.end.column, ), ), ); diff --git a/pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart b/pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart index 2f4d8bd..dff3847 100644 --- a/pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart +++ b/pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart @@ -308,13 +308,16 @@ class FindReferencesVisitor } var component = complexSelector.components.first; - var selector = component.selector; - var name = selector.span.text; + var selectorSpan = component.selector.span; + var name = selectorSpan.text; if (!name.contains(_name)) { continue; } - var nameRange = selectorNameRange(node, selector); + var nameRange = selectorNameRange( + node: node.span, + selector: selectorSpan, + ); candidates.add( Reference( diff --git a/pkgs/sass_language_services/lib/src/features/go_to_definition/scope_visitor.dart b/pkgs/sass_language_services/lib/src/features/go_to_definition/scope_visitor.dart index 6dadf7c..74600cf 100644 --- a/pkgs/sass_language_services/lib/src/features/go_to_definition/scope_visitor.dart +++ b/pkgs/sass_language_services/lib/src/features/go_to_definition/scope_visitor.dart @@ -67,11 +67,12 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { if (isCustomProperty) { // Add all custom properties to the global scope. + var nameSpan = node.name.span; var range = toRange(node.span); - var selectionRange = toRange(node.name.span); + var selectionRange = toRange(nameSpan); var symbol = StylesheetDocumentSymbol( - name: node.name.span.text, + name: nameSpan.text, referenceKind: ReferenceKind.customProperty, range: range, children: [], @@ -86,24 +87,26 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { @override void visitEachRule(sass.EachRule node) { - var lengthSubtract = node.list.span.end.offset - node.span.start.offset; + var span = node.span; + var listSpan = node.list.span; + var lengthSubtract = listSpan.end.offset - span.start.offset; var scope = _addScope( - offset: node.list.span.end.offset, - length: node.span.length - lengthSubtract, + offset: listSpan.end.offset, + length: span.length - lengthSubtract, ); if (scope != null) { for (var variable in node.variables) { - var variableIndex = node.span.text.indexOf(variable); + var variableIndex = span.text.indexOf(variable); - var range = toRange(node.span); + var range = toRange(span); var selectionRange = lsp.Range( start: lsp.Position( - line: node.span.start.line, - character: node.span.start.column + variableIndex), + line: span.start.line, + character: span.start.column + variableIndex), end: lsp.Position( - line: node.span.start.line, - character: node.span.start.column + variable.length, + line: span.start.line, + character: span.start.column + variable.length, ), ); @@ -123,24 +126,25 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { @override void visitForRule(sass.ForRule node) { - var toEndIndex = node.to.span.end.offset - node.span.start.offset; - var scopeIndex = node.span.text.indexOf(openBracketOrNewline, toEndIndex); + var span = node.span; + var toEndIndex = node.to.span.end.offset - span.start.offset; + var scopeIndex = span.text.indexOf(openBracketOrNewline, toEndIndex); var scope = _addScope( - offset: node.span.start.offset + scopeIndex, - length: node.span.length - scopeIndex, + offset: span.start.offset + scopeIndex, + length: span.length - scopeIndex, ); if (scope != null) { - var variableIndex = node.span.text.indexOf(node.variable); + var variableIndex = span.text.indexOf(node.variable); - var range = toRange(node.span); + var range = toRange(span); var selectionRange = lsp.Range( start: lsp.Position( - line: node.span.start.line, - character: node.span.start.column + variableIndex), + line: span.start.line, + character: span.start.column + variableIndex), end: lsp.Position( - line: node.span.start.line, - character: node.span.start.column + node.variable.length, + line: span.start.line, + character: span.start.column + node.variable.length, ), ); @@ -159,20 +163,21 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { @override void visitFunctionRule(node) { + var span = node.span; _addSymbol( name: node.name, kind: ReferenceKind.function, - symbolRange: toRange(node.span), + symbolRange: toRange(span), nameRange: toRange(node.nameSpan), - offset: node.span.start.offset, - length: node.span.length, + offset: span.start.offset, + length: span.length, ); - var argsEndIndex = node.arguments.span.end.offset - node.span.start.offset; - var scopeIndex = node.span.text.indexOf(openBracketOrNewline, argsEndIndex); + var argsEndIndex = node.arguments.span.end.offset - span.start.offset; + var scopeIndex = span.text.indexOf(openBracketOrNewline, argsEndIndex); var scope = _addScope( - offset: node.span.start.offset + scopeIndex, - length: node.span.length - scopeIndex, + offset: span.start.offset + scopeIndex, + length: span.length - scopeIndex, ); if (scope != null) { @@ -196,37 +201,40 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { @override void visitIfRule(sass.IfRule node) { // TODO: would be nice to have the spans for clauses from sass_api. + var span = node.span; Scope? previousClause; for (var clause in node.clauses) { - var argsEndIndex = - clause.expression.span.end.offset - node.span.start.offset; + var argsEndIndex = clause.expression.span.end.offset - span.start.offset; var scopeStartIndex = - node.span.text.indexOf(openBracketOrNewline, argsEndIndex); + span.text.indexOf(openBracketOrNewline, argsEndIndex); var toMatch = dialect == Dialect.indented ? '\n' : '}'; - var lastChildIndex = - node.span.text.indexOf(clause.children.last.span.text); - var scopeEndIndex = node.span.text.indexOf( - toMatch, lastChildIndex + clause.children.last.span.text.length); + var lastChildSpan = clause.children.last.span; + var lastChildIndex = span.text.indexOf(lastChildSpan.text); + var scopeEndIndex = span.text.indexOf( + toMatch, + lastChildIndex + lastChildSpan.text.length, + ); previousClause = _addScope( - offset: node.span.start.offset + scopeStartIndex, + offset: span.start.offset + scopeStartIndex, length: scopeEndIndex - scopeStartIndex + 1, ); } if (previousClause != null && node.lastClause != null) { - var scopeIndex = node.span.text.indexOf( - openBracketOrNewline, - previousClause.offset - - node.span.start.offset + - previousClause.length + - "@else".length); + var scopeIndex = span.text.indexOf( + openBracketOrNewline, + previousClause.offset - + span.start.offset + + previousClause.length + + "@else".length, + ); _addScope( - offset: node.span.start.offset + scopeIndex, - length: node.span.length - scopeIndex, + offset: span.start.offset + scopeIndex, + length: span.length - scopeIndex, ); } @@ -235,20 +243,21 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { @override void visitMixinRule(node) { + var span = node.span; _addSymbol( name: node.name, kind: ReferenceKind.mixin, - symbolRange: toRange(node.span), + symbolRange: toRange(span), nameRange: toRange(node.nameSpan), - offset: node.span.start.offset, - length: node.span.length, + offset: span.start.offset, + length: span.length, ); - var argsEndIndex = node.arguments.span.end.offset - node.span.start.offset; - var scopeIndex = node.span.text.indexOf(openBracketOrNewline, argsEndIndex); + var argsEndIndex = node.arguments.span.end.offset - span.start.offset; + var scopeIndex = span.text.indexOf(openBracketOrNewline, argsEndIndex); var scope = _addScope( - offset: node.span.start.offset + scopeIndex, - length: node.span.length - scopeIndex, + offset: span.start.offset + scopeIndex, + length: span.length - scopeIndex, ); if (scope != null) { @@ -274,6 +283,8 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { if (node.selector.isPlain) { try { var selectorList = sass.SelectorList.parse(node.selector.asPlain!); + + var span = node.span; for (var complexSelector in selectorList.components) { // we only want selectors that can be used in @extend if (complexSelector.components.isEmpty || @@ -283,9 +294,10 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { var component = complexSelector.components.first; var selector = component.selector; - var name = selector.span.text; + var selectorSpan = selector.span; + var name = selectorSpan.text; - var nameRange = selectorNameRange(node, selector); + var nameRange = selectorNameRange(node: span, selector: selectorSpan); // symbolRange: start position of selector's nameRange, end of stylerule (node.span.end). var symbolRange = lsp.Range( @@ -294,8 +306,8 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { character: nameRange.start.character, ), end: lsp.Position( - line: node.span.end.line, - character: node.span.end.column, + line: span.end.line, + character: span.end.column, ), ); @@ -306,14 +318,15 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { : ReferenceKind.selector, symbolRange: symbolRange, nameRange: nameRange, - offset: node.span.start.offset, - length: node.span.length, + offset: span.start.offset, + length: span.length, ); } + var selectorsLength = node.selector.span.length; _addScope( - offset: node.span.start.offset + node.selector.span.length, - length: node.span.length - node.selector.span.length, + offset: span.start.offset + selectorsLength, + length: span.length - selectorsLength, ); } on sass.SassFormatException catch (_) { // Do nothing. @@ -325,10 +338,11 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { @override void visitVariableDeclaration(node) { + var span = node.span; _addSymbol( name: node.name, kind: ReferenceKind.variable, - symbolRange: toRange(node.span), + symbolRange: toRange(span), nameRange: lsp.Range( start: lsp.Position( line: node.nameSpan.start.line, @@ -340,8 +354,8 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { character: node.nameSpan.end.column, ), ), - offset: node.span.start.offset, - length: node.span.length, + offset: span.start.offset, + length: span.length, ); super.visitVariableDeclaration(node); @@ -349,14 +363,13 @@ class ScopeVisitor with sass.RecursiveStatementVisitor { @override void visitWhileRule(sass.WhileRule node) { - var conditionEndIndex = - node.condition.span.end.offset - node.span.start.offset; - var scopeIndex = - node.span.text.indexOf(openBracketOrNewline, conditionEndIndex); + var span = node.span; + var conditionEndIndex = node.condition.span.end.offset - span.start.offset; + var scopeIndex = span.text.indexOf(openBracketOrNewline, conditionEndIndex); _addScope( - offset: node.span.start.offset + scopeIndex, - length: node.span.length - scopeIndex, + offset: span.start.offset + scopeIndex, + length: span.length - scopeIndex, ); super.visitWhileRule(node); diff --git a/pkgs/sass_language_services/lib/src/features/node_at_offset_visitor.dart b/pkgs/sass_language_services/lib/src/features/node_at_offset_visitor.dart index 01a40ac..2f42737 100644 --- a/pkgs/sass_language_services/lib/src/features/node_at_offset_visitor.dart +++ b/pkgs/sass_language_services/lib/src/features/node_at_offset_visitor.dart @@ -22,18 +22,27 @@ class NodeAtOffsetVisitor NodeAtOffsetVisitor(int offset) : _offset = offset; sass.AstNode? _process(sass.AstNode node) { - var containsOffset = - node.span.start.offset <= _offset && node.span.end.offset >= _offset; + var nodeSpan = node.span; + var nodeStartOffset = nodeSpan.start.offset; + var nodeEndOffset = nodeSpan.end.offset; + var containsOffset = nodeStartOffset <= _offset && nodeEndOffset >= _offset; if (containsOffset) { if (candidate == null) { candidate = node; - } else if (node.span.length <= candidate!.span.length) { - candidate = node; + } else { + var nodeLength = nodeEndOffset - nodeStartOffset; + // store candidateSpan next to _candidate + var candidateSpan = candidate!.span; + var candidateLength = + candidateSpan.end.offset - candidateSpan.start.offset; + if (nodeLength <= candidateLength) { + candidate = node; + } } } - if (node.span.start.offset > _offset) { + if (nodeStartOffset > _offset) { return candidate; } diff --git a/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart b/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart index bef038f..1956230 100644 --- a/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart +++ b/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart @@ -12,34 +12,35 @@ lsp.Range toRange(FileSpan span) { } lsp.Range selectorNameRange( - sass.StyleRule node, sass.CompoundSelector selector) { + {required FileSpan node, required FileSpan selector}) { // The selector span seems to be relative to node, not to the file. return lsp.Range( start: lsp.Position( - line: node.span.start.line + selector.span.start.line, - character: node.span.start.column + selector.span.start.column, + line: node.start.line + selector.start.line, + character: node.start.column + selector.start.column, ), end: lsp.Position( - line: node.span.start.line + selector.span.end.line, - character: node.span.start.column + selector.span.end.column, + line: node.start.line + selector.end.line, + character: node.start.column + selector.end.column, ), ); } lsp.Range forwardVisibilityRange(sass.ForwardRule node, String name) { - var nameIndex = node.span.text.indexOf( + var span = node.span; + var nameIndex = span.text.indexOf( name, - node.span.start.offset + node.urlSpan.end.offset, + span.start.offset + node.urlSpan.end.offset, ); var selectionRange = lsp.Range( start: lsp.Position( - line: node.span.start.line, - character: node.span.start.column + nameIndex, + line: span.start.line, + character: span.start.column + nameIndex, ), end: lsp.Position( - line: node.span.start.line, - character: node.span.start.column + nameIndex + name.length, + line: span.start.line, + character: span.start.column + nameIndex + name.length, ), ); return selectionRange; From 9f95d729ed12f3d5df520f697d58cc285a230f0d Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 24 Nov 2024 12:21:22 +0100 Subject: [PATCH 8/8] Document highlights --- .../definition/document-highlights.test.js | 52 +++++++++++++++++++ .../electron/definition/fixtures/styles.scss | 8 +++ .../document_highlights_feature.dart | 15 +++--- .../go_to_definition/scoped_symbols.dart | 21 ++++++-- 4 files changed, 86 insertions(+), 10 deletions(-) create mode 100644 extension/test/electron/definition/document-highlights.test.js diff --git a/extension/test/electron/definition/document-highlights.test.js b/extension/test/electron/definition/document-highlights.test.js new file mode 100644 index 0000000..695ac19 --- /dev/null +++ b/extension/test/electron/definition/document-highlights.test.js @@ -0,0 +1,52 @@ +const assert = require('node:assert'); +const path = require('node:path'); +const vscode = require('vscode'); +const { showFile, sleepCI } = require('../util'); + +const stylesUri = vscode.Uri.file( + path.resolve(__dirname, 'fixtures', 'styles.scss') +); + +before(async () => { + await showFile(stylesUri); + await sleepCI(); +}); + +after(async () => { + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); +}); + +/** + * @param {import('vscode').Uri} documentUri + * @returns {Promise>} + */ +async function getHighlights(documentUri, position) { + const result = await vscode.commands.executeCommand( + 'vscode.executeDocumentHighlights', + documentUri, + position + ); + return result; +} + +test('gets document highlights', async () => { + const [first, second] = await getHighlights( + stylesUri, + new vscode.Position(7, 5) + ); + + assert.ok(first, 'Should have found highlights'); + assert.ok(second, 'Should have found two highlights'); + + assert.equal(first.range.start.line, 7); + assert.equal(first.range.start.character, 2); + + assert.equal(first.range.end.line, 7); + assert.equal(first.range.end.character, 14); + + assert.equal(second.range.start.line, 11); + assert.equal(second.range.start.character, 13); + + assert.equal(second.range.end.line, 11); + assert.equal(second.range.end.character, 25); +}); diff --git a/extension/test/electron/definition/fixtures/styles.scss b/extension/test/electron/definition/fixtures/styles.scss index de048e2..bd80a97 100644 --- a/extension/test/electron/definition/fixtures/styles.scss +++ b/extension/test/electron/definition/fixtures/styles.scss @@ -3,3 +3,11 @@ .a { @extend %brand; } + +:root { + --color-text: #000; +} + +body { + color: var(--color-text); +} diff --git a/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart b/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart index dd59eae..15e90b0 100644 --- a/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/document_highlights/document_highlights_feature.dart @@ -2,7 +2,6 @@ import 'package:lsp_server/lsp_server.dart' as lsp; import 'package:sass_api/sass_api.dart' as sass; import 'package:sass_language_services/sass_language_services.dart'; import 'package:sass_language_services/src/features/find_references/find_references_visitor.dart'; -import 'package:sass_language_services/src/utils/sass_lsp_utils.dart'; import '../go_to_definition/scope_visitor.dart'; import '../go_to_definition/scoped_symbols.dart'; @@ -39,17 +38,21 @@ class DocumentHighlightsFeature extends LanguageFeature { var symbol = symbols.findSymbolFromNode(node); var result = []; - var references = FindReferencesVisitor( + var visitor = FindReferencesVisitor( document, name, includeDeclaration: true, ); - for (var reference in references.candidates) { + stylesheet.accept(visitor); + + for (var reference in visitor.candidates) { if (symbol != null) { - if (symbol.referenceKind == reference.kind && - symbol.name == reference.name && - isSameRange(symbol.range, reference.location.range)) { + if (symbols.matchesSymbol( + reference, + document.offsetAt(reference.location.range.start), + symbol, + )) { result.add( lsp.DocumentHighlight(range: reference.location.range), ); diff --git a/pkgs/sass_language_services/lib/src/features/go_to_definition/scoped_symbols.dart b/pkgs/sass_language_services/lib/src/features/go_to_definition/scoped_symbols.dart index f8dd92d..9f5cbab 100644 --- a/pkgs/sass_language_services/lib/src/features/go_to_definition/scoped_symbols.dart +++ b/pkgs/sass_language_services/lib/src/features/go_to_definition/scoped_symbols.dart @@ -1,6 +1,8 @@ import 'package:sass_api/sass_api.dart' as sass; +import 'package:sass_language_services/src/utils/sass_lsp_utils.dart'; import '../document_symbols/stylesheet_document_symbol.dart'; +import '../find_references/reference.dart'; import 'scope.dart'; import 'scope_visitor.dart'; @@ -123,20 +125,20 @@ class ScopedSymbols { var referenceKind = getNodeReferenceKind(node); if (referenceKind != null) { - return _findSymbol(node, referenceKind); + return _findSymbol( + getNodeName(node), node.span.start.offset, referenceKind); } return null; } StylesheetDocumentSymbol? _findSymbol( - sass.AstNode node, ReferenceKind referenceKind) { - var name = getNodeName(node); + String? name, int offset, ReferenceKind referenceKind) { if (name == null) { return null; } - var scope = globalScope.findScope(offset: node.span.start.offset); + var scope = globalScope.findScope(offset: offset); while (scope != null) { var symbol = scope.getSymbol(name: name, referenceKind: referenceKind); if (symbol != null) { @@ -167,4 +169,15 @@ class ScopedSymbols { } return result; } + + bool matchesSymbol( + Reference reference, int offset, StylesheetDocumentSymbol symbol) { + var referenceSymbol = _findSymbol(reference.name, offset, reference.kind); + if (referenceSymbol == null) { + return false; + } + return referenceSymbol.name == symbol.name && + referenceSymbol.kind == symbol.kind && + isSameRange(referenceSymbol.range, symbol.range); + } }