From e58c76106d93c4ccd5afd17290257bbd9394ae1a Mon Sep 17 00:00:00 2001 From: William Killerud Date: Tue, 5 Nov 2024 20:38:40 +0100 Subject: [PATCH 01/23] Document symbols feature --- .../document_symbols_feature.dart | 16 ++++++ .../document_symbols_visitor.dart | 43 +++++++++++++++ .../stylesheet_document_symbol.dart | 17 ++++++ .../stylesheet_document_symbols.dart | 43 +++++++++++++++ .../lib/src/language_services.dart | 19 +++++-- .../lib/src/utils/sass_lsp_utils.dart | 11 ++++ pkgs/sass_language_services/pubspec.yaml | 1 + .../document_symbols_test.dart | 55 +++++++++++++++++++ 8 files changed, 199 insertions(+), 6 deletions(-) create mode 100644 pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_feature.dart create mode 100644 pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_visitor.dart create mode 100644 pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart create mode 100644 pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart create mode 100644 pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart create mode 100644 pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart 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 new file mode 100644 index 0000000..4631b0a --- /dev/null +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_feature.dart @@ -0,0 +1,16 @@ +import 'package:sass_language_services/sass_language_services.dart'; + +import '../language_feature.dart'; +import 'document_symbols_visitor.dart'; +import 'stylesheet_document_symbols.dart'; + +class DocumentSymbolsFeature extends LanguageFeature { + DocumentSymbolsFeature({required super.ls}); + + StylesheetDocumentSymbols findDocumentSymbols(TextDocument document) { + var stylesheet = ls.parseStylesheet(document); + var symbolsVisitor = DocumentSymbolsVisitor(); + stylesheet.accept(symbolsVisitor); + return symbolsVisitor.symbols; + } +} 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 new file mode 100644 index 0000000..cf9afb9 --- /dev/null +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_visitor.dart @@ -0,0 +1,43 @@ +import 'package:lsp_server/lsp_server.dart' as lsp; +import 'package:sass_api/sass_api.dart' as sass; +import 'package:sass_language_services/src/utils/sass_lsp_utils.dart'; + +import './stylesheet_document_symbols.dart'; +import 'stylesheet_document_symbol.dart'; + +final quotes = RegExp('["\']'); + +class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { + final symbols = StylesheetDocumentSymbols(); + + @override + void visitStyleRule(sass.StyleRule node) { + super.visitStyleRule(node); + + if (!node.selector.isPlain) { + // Keeping it simple for now. + return; + } + + try { + var selectorList = sass.SelectorList.parse(node.selector.asPlain!); + for (var complexSelector in selectorList.components) { + for (var component in complexSelector.components) { + for (var simpleSelector in component.selector.components) { + var name = simpleSelector.toString(); + + var symbol = StylesheetDocumentSymbol( + name: name.trim(), + kind: lsp.SymbolKind.Class, + range: toRange(node.span), + selectionRange: toRange(node.span)); + + symbols.classes.add(symbol); + } + } + } + } on sass.SassFormatException catch (_) { + // Do nothing. + } + } +} diff --git a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart new file mode 100644 index 0000000..ee84c52 --- /dev/null +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart @@ -0,0 +1,17 @@ +import 'package:lsp_server/lsp_server.dart'; + +class StylesheetDocumentSymbol extends DocumentSymbol { + final String? documentation; + + StylesheetDocumentSymbol({ + required super.name, + required super.kind, + required super.range, + required super.selectionRange, + super.detail, + super.children, + super.tags, + super.deprecated, + this.documentation, + }); +} diff --git a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart new file mode 100644 index 0000000..ddb2542 --- /dev/null +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart @@ -0,0 +1,43 @@ +import 'package:sass_language_services/src/features/document_symbols/stylesheet_document_symbol.dart'; + +class StylesheetDocumentSymbols { + /// Sass variable declarations. + final variables = []; + + /// Sass function declarations. + final functions = []; + + /// Sass mixin declarations. + final mixins = []; + + /// Placeholder selectors this document declares. + final placeholders = []; + + /// Placeholder selectors this document @extends. + final placeholderUsages = []; + + /// Declared CSS selectors. + final classes = []; + + /// Declared CSS variables. + final cssVariables = []; + + final keyframeIdentifiers = []; + + final fontFaces = []; + + final mediaQueries = []; + + List get symbols => [ + ...variables, + ...functions, + ...mixins, + ...placeholders, + ...placeholderUsages, + ...classes, + ...cssVariables, + ...keyframeIdentifiers, + ...fontFaces, + ...mediaQueries + ]; +} diff --git a/pkgs/sass_language_services/lib/src/language_services.dart b/pkgs/sass_language_services/lib/src/language_services.dart index f9e58d6..454a47e 100644 --- a/pkgs/sass_language_services/lib/src/language_services.dart +++ b/pkgs/sass_language_services/lib/src/language_services.dart @@ -2,25 +2,28 @@ 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 'features/document_symbols/document_symbols_feature.dart'; +import 'features/document_symbols/stylesheet_document_symbols.dart'; import 'features/document_links/document_links_feature.dart'; import 'language_services_cache.dart'; class LanguageServices { - late final LanguageServicesCache cache; + final LanguageServicesCache cache; final lsp.ClientCapabilities clientCapabilities; final FileSystemProvider fs; LanguageServerConfiguration configuration = LanguageServerConfiguration.create(null); - late final DocumentLinksFeature _links; + late final DocumentLinksFeature _documentLinks; + late final DocumentSymbolsFeature _documentSymbols; LanguageServices({ required this.clientCapabilities, required this.fs, - }) { - cache = LanguageServicesCache(); - _links = DocumentLinksFeature(ls: this); + }) : cache = LanguageServicesCache() { + _documentLinks = DocumentLinksFeature(ls: this); + _documentSymbols = DocumentSymbolsFeature(ls: this); } void configure(LanguageServerConfiguration configuration) { @@ -29,7 +32,11 @@ class LanguageServices { Future> findDocumentLinks( TextDocument document) { - return _links.findDocumentLinks(document); + return _documentLinks.findDocumentLinks(document); + } + + StylesheetDocumentSymbols findDocumentSymbols(TextDocument document) { + return _documentSymbols.findDocumentSymbols(document); } sass.Stylesheet parseStylesheet(TextDocument 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 new file mode 100644 index 0000000..f3c3b5b --- /dev/null +++ b/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart @@ -0,0 +1,11 @@ +import 'package:lsp_server/lsp_server.dart' as lsp; +import 'package:source_span/source_span.dart'; + +lsp.Range toRange(FileSpan span) { + return lsp.Range( + start: lsp.Position( + line: span.start.line, + character: span.start.column, + ), + end: lsp.Position(line: span.end.line, character: span.end.column)); +} diff --git a/pkgs/sass_language_services/pubspec.yaml b/pkgs/sass_language_services/pubspec.yaml index e9462e4..b4dd0aa 100644 --- a/pkgs/sass_language_services/pubspec.yaml +++ b/pkgs/sass_language_services/pubspec.yaml @@ -11,6 +11,7 @@ dependencies: lsp_server: ^0.3.2 path: ^1.9.0 sass_api: ^14.1.0 + source_span: ^1.10.0 dev_dependencies: lints: ^4.0.0 diff --git a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart new file mode 100644 index 0000000..e8c76e8 --- /dev/null +++ b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart @@ -0,0 +1,55 @@ +import 'package:sass_language_services/sass_language_services.dart'; +import 'package:test/test.dart'; + +import '../../memory_file_system.dart'; +import '../../test_client_capabilities.dart'; + +final fs = MemoryFileSystem(); +final ls = LanguageServices(fs: fs, clientCapabilities: getCapabilities()); + +void main() { + group('Document symbols', () { + setUp(() { + ls.cache.clear(); + }); + + test('should collect CSS class selectors', () async { + var document = fs.createDocument(''' +.foo { + color: red; +} + +.bar { + color: blue; +} +'''); + + var result = ls.findDocumentSymbols(document); + expect(result.symbols.length, equals(2)); + + expect(result.classes.first.name, equals(".foo")); + expect(result.classes.last.name, equals(".bar")); + }); + + test('should collect individual CSS class selectors when combined', + () async { + var document = fs.createDocument(''' +.foo.bar { + color: red; +} + +.fizz .buzz { + color: blue; +} +'''); + + var result = ls.findDocumentSymbols(document); + expect(result.symbols.length, equals(4)); + + expect(result.classes[0].name, equals(".foo")); + expect(result.classes[1].name, equals(".bar")); + expect(result.classes[2].name, equals(".fizz")); + expect(result.classes[3].name, equals(".buzz")); + }); + }); +} From 2f970fe47de10bf5e8744cb1ea89eb694cdc8f15 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Thu, 7 Nov 2024 20:08:48 +0100 Subject: [PATCH 02/23] Fix simpleSelector range [skip ci] --- .../document_symbols/document_symbols_visitor.dart | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) 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 cf9afb9..1c5ef46 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 @@ -24,13 +24,11 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { for (var complexSelector in selectorList.components) { for (var component in complexSelector.components) { for (var simpleSelector in component.selector.components) { - var name = simpleSelector.toString(); - var symbol = StylesheetDocumentSymbol( - name: name.trim(), + name: simpleSelector.toString(), kind: lsp.SymbolKind.Class, - range: toRange(node.span), - selectionRange: toRange(node.span)); + range: toRange(simpleSelector.span), + selectionRange: toRange(simpleSelector.span)); symbols.classes.add(symbol); } From f6bed440a4f0afc866d7b73edf865731e2e218a8 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Fri, 8 Nov 2024 20:32:48 +0100 Subject: [PATCH 03/23] Fix behavior for combined selectors VSCode's approach is to treat a selector as one whole. --- .../document_symbols_visitor.dart | 16 +++---- .../stylesheet_document_symbols.dart | 4 +- .../document_symbols_test.dart | 45 ++++++++++++++----- 3 files changed, 42 insertions(+), 23 deletions(-) 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 1c5ef46..7e4fb63 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 @@ -22,17 +22,13 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { try { var selectorList = sass.SelectorList.parse(node.selector.asPlain!); for (var complexSelector in selectorList.components) { - for (var component in complexSelector.components) { - for (var simpleSelector in component.selector.components) { - var symbol = StylesheetDocumentSymbol( - name: simpleSelector.toString(), - kind: lsp.SymbolKind.Class, - range: toRange(simpleSelector.span), - selectionRange: toRange(simpleSelector.span)); + var symbol = StylesheetDocumentSymbol( + name: complexSelector.span.text.trim(), + kind: lsp.SymbolKind.Class, + range: toRange(complexSelector.span), + selectionRange: toRange(complexSelector.span)); - symbols.classes.add(symbol); - } - } + symbols.selectors.add(symbol); } } on sass.SassFormatException catch (_) { // Do nothing. diff --git a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart index ddb2542..a0dec7c 100644 --- a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart @@ -17,7 +17,7 @@ class StylesheetDocumentSymbols { final placeholderUsages = []; /// Declared CSS selectors. - final classes = []; + final selectors = []; /// Declared CSS variables. final cssVariables = []; @@ -34,7 +34,7 @@ class StylesheetDocumentSymbols { ...mixins, ...placeholders, ...placeholderUsages, - ...classes, + ...selectors, ...cssVariables, ...keyframeIdentifiers, ...fontFaces, diff --git a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart index e8c76e8..8f9a4a4 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart @@ -8,12 +8,12 @@ final fs = MemoryFileSystem(); final ls = LanguageServices(fs: fs, clientCapabilities: getCapabilities()); void main() { - group('Document symbols', () { + group('selectors', () { setUp(() { ls.cache.clear(); }); - test('should collect CSS class selectors', () async { + test('should collect CSS class selectors', () { var document = fs.createDocument(''' .foo { color: red; @@ -27,12 +27,11 @@ void main() { var result = ls.findDocumentSymbols(document); expect(result.symbols.length, equals(2)); - expect(result.classes.first.name, equals(".foo")); - expect(result.classes.last.name, equals(".bar")); + expect(result.selectors.first.name, equals(".foo")); + expect(result.selectors.last.name, equals(".bar")); }); - test('should collect individual CSS class selectors when combined', - () async { + test('should treat CSS selectors with multiple classes as one', () { var document = fs.createDocument(''' .foo.bar { color: red; @@ -44,12 +43,36 @@ void main() { '''); var result = ls.findDocumentSymbols(document); - expect(result.symbols.length, equals(4)); + expect(result.symbols.length, equals(2)); + + expect(result.selectors.first.name, equals(".foo.bar")); + expect(result.selectors.last.name, equals(".fizz .buzz")); + }); + + test('should treat lists of selectors as separate', () { + var document = fs.createDocument(''' +.foo.bar, +.fizz .buzz { + color: red; +} +'''); - expect(result.classes[0].name, equals(".foo")); - expect(result.classes[1].name, equals(".bar")); - expect(result.classes[2].name, equals(".fizz")); - expect(result.classes[3].name, equals(".buzz")); + var result = ls.findDocumentSymbols(document); + expect(result.symbols.length, equals(2)); + + expect(result.selectors.first.name, equals(".foo.bar")); + expect(result.selectors.last.name, equals(".fizz .buzz")); + }); + + test('should include extras', () { + var document = fs.createDocument(''' +.foo:has([data-testid="bar"]) { + color: red; +} +'''); + var result = ls.findDocumentSymbols(document); + expect( + result.selectors.first.name, equals('.foo:has([data-testid="bar"])')); }); }); } From 3bd6f5979e95bc56b498ded46df68ae426351250 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Fri, 8 Nov 2024 21:17:07 +0100 Subject: [PATCH 04/23] Test selector symbols end to end --- extension/package.json | 41 ++++++++++++++----- extension/test/README.md | 25 +++++++++++ .../document-symbols/document-symbols.test.js | 39 ++++++++++++++++++ .../document-symbols/fixtures/styles.sass | 17 ++++++++ .../test/electron/document-symbols/index.js | 25 +++++++++++ .../lib/src/language_server.dart | 26 +++++++++++- .../configuration/language_configuration.dart | 20 ++++----- .../document_symbols_feature.dart | 2 +- .../document_symbols_visitor.dart | 9 +++- .../stylesheet_document_symbol.dart | 7 +--- 10 files changed, 181 insertions(+), 30 deletions(-) create mode 100644 extension/test/README.md create mode 100644 extension/test/electron/document-symbols/document-symbols.test.js create mode 100644 extension/test/electron/document-symbols/fixtures/styles.sass create mode 100644 extension/test/electron/document-symbols/index.js diff --git a/extension/package.json b/extension/package.json index e1998c2..7f8e7c5 100644 --- a/extension/package.json +++ b/extension/package.json @@ -213,6 +213,13 @@ "default": true, "description": "Enable or disable all diagnostics." }, + "sass.scss.documentSymbols.enabled": { + "order": 55, + "type": "boolean", + "scope": "resource", + "default": true, + "description": "Enable or disable Document symbols." + }, "sass.scss.foldingRanges.enabled": { "order": 60, "type": "boolean", @@ -248,7 +255,7 @@ "default": true, "description": "Show references to Sass documentation for Sass built-in modules and SassDoc for annotations." }, - "sass.scss.links.enabled": { + "sass.scss.documentLinks.enabled": { "order": 90, "type": "boolean", "scope": "resource", @@ -283,7 +290,7 @@ "default": true, "description": "Enable or disable signature help." }, - "sass.scss.workspaceSymbol.enabled": { + "sass.scss.workspaceSymbols.enabled": { "order": 140, "type": "boolean", "scope": "resource", @@ -371,6 +378,13 @@ "default": true, "description": "Enable or disable all diagnostics." }, + "sass.sass.documentSymbols.enabled": { + "order": 55, + "type": "boolean", + "scope": "resource", + "default": true, + "description": "Enable or disable Document symbols." + }, "sass.sass.foldingRanges.enabled": { "order": 60, "type": "boolean", @@ -406,7 +420,7 @@ "default": true, "description": "Show references to MDN in CSS hovers, Sass documentation for Sass built-in modules and SassDoc for annotations." }, - "sass.sass.links.enabled": { + "sass.sass.documentLinks.enabled": { "order": 90, "type": "boolean", "scope": "resource", @@ -439,14 +453,14 @@ "type": "boolean", "scope": "resource", "default": true, - "description": "Enable or disable selection ranges." + "description": "Enable or disable signature help." }, - "sass.sass.workspaceSymbol.enabled": { + "sass.sass.workspaceSymbols.enabled": { "order": 140, "type": "boolean", "scope": "resource", "default": true, - "description": "Enable or disable selection ranges." + "description": "Enable or disable workspace symbols." } } }, @@ -502,6 +516,13 @@ "default": false, "description": "Enable or disable all diagnostics." }, + "sass.css.documentSymbols.enabled": { + "order": 55, + "type": "boolean", + "scope": "resource", + "default": true, + "description": "Enable or disable Document symbols." + }, "sass.css.foldingRanges.enabled": { "order": 60, "type": "boolean", @@ -537,7 +558,7 @@ "default": true, "description": "Show references to MDN in CSS hovers." }, - "sass.css.links.enabled": { + "sass.css.documentLinks.enabled": { "order": 90, "type": "boolean", "scope": "resource", @@ -570,14 +591,14 @@ "type": "boolean", "scope": "resource", "default": false, - "description": "Enable or disable selection ranges." + "description": "Enable or disable signature help." }, - "sass.css.workspaceSymbol.enabled": { + "sass.css.workspaceSymbols.enabled": { "order": 140, "type": "boolean", "scope": "resource", "default": false, - "description": "Enable or disable selection ranges." + "description": "Enable or disable workspace symbols." } } } diff --git a/extension/test/README.md b/extension/test/README.md new file mode 100644 index 0000000..51c2d77 --- /dev/null +++ b/extension/test/README.md @@ -0,0 +1,25 @@ +# Testing the VS Code extension + +These tests automate Visual Studio Code using the [VS Code JavaScript API](https://code.visualstudio.com/api/references/vscode-api). +See [the list of built-in commands](https://code.visualstudio.com/api/references/commands#commands) to see how you can automate the interactions you need for testing. + +There are two different test runners available: + +- [@vscode/test-electron](https://github.com/microsoft/vscode-test) +- [@vscode/test-web](https://github.com/microsoft/vscode-test-web) + +We only test using Electron. + +The Electron runner is configured so it can run multiple instances of VS Code (in sequence) using +different configurations and workspaces. Each subdirectory in `electron/` will run in a separate instance +of VS Code. + +By convention subdirectories in `e2e/` must have: + +- `index.js` as the entrypoint that finds test files and passes them to Mocha +- at least one `*.test.js` file with some tests +- a `fixtures/` subdirectory with: + - `.vscode/settings.json` + - `styles.scss` or `styles.sass` + +Starting and stopping VS Code takes a few seconds, so try to use the same workspaces as much as possible. diff --git a/extension/test/electron/document-symbols/document-symbols.test.js b/extension/test/electron/document-symbols/document-symbols.test.js new file mode 100644 index 0000000..067b6d1 --- /dev/null +++ b/extension/test/electron/document-symbols/document-symbols.test.js @@ -0,0 +1,39 @@ +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.sass') +); + +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 findDocumentSymbols(documentUri) { + const links = await vscode.commands.executeCommand( + 'vscode.executeDocumentSymbolProvider', + documentUri + ); + return links; +} + +test('gets CSS selectors', async () => { + await showFile(stylesUri); + const result = await findDocumentSymbols(stylesUri); + + assert.ok( + result.find((s) => s.name === '.card .body:has(:not(.stuff))'), + 'Should have found .card .body:has(:not(.stuff))' + ); +}); diff --git a/extension/test/electron/document-symbols/fixtures/styles.sass b/extension/test/electron/document-symbols/fixtures/styles.sass new file mode 100644 index 0000000..d684949 --- /dev/null +++ b/extension/test/electron/document-symbols/fixtures/styles.sass @@ -0,0 +1,17 @@ +button + border-color: red + +.hello + color: green + +.world + color: blue + +.card > .header +.card > .body +.card > .footer + padding: 2px + + +.card .body:has(:not(.stuff)) + padding: 4px diff --git a/extension/test/electron/document-symbols/index.js b/extension/test/electron/document-symbols/index.js new file mode 100644 index 0000000..83212dd --- /dev/null +++ b/extension/test/electron/document-symbols/index.js @@ -0,0 +1,25 @@ +const path = require('node:path'); +const fs = require('node:fs/promises'); +const vscode = require('vscode'); +const { runMocha } = require('../mocha'); + +/** + * @returns {Promise} + */ +async function run() { + const filePaths = []; + + const dir = await fs.readdir(__dirname, { withFileTypes: true }); + for (let entry of dir) { + if (entry.isFile() && entry.name.endsWith('test.js')) { + filePaths.push(path.join(entry.parentPath, entry.name)); + } + } + + await runMocha( + filePaths, + vscode.Uri.file(path.resolve(__dirname, 'fixtures', 'styles.sass')) + ); +} + +module.exports = { run }; diff --git a/pkgs/sass_language_server/lib/src/language_server.dart b/pkgs/sass_language_server/lib/src/language_server.dart index 1cf0210..614f265 100644 --- a/pkgs/sass_language_server/lib/src/language_server.dart +++ b/pkgs/sass_language_server/lib/src/language_server.dart @@ -168,6 +168,7 @@ class LanguageServer { var serverCapabilities = ServerCapabilities( documentLinkProvider: DocumentLinkOptions(resolveProvider: false), + documentSymbolProvider: Either2.t1(true), textDocumentSync: Either2.t1(TextDocumentSyncKind.Incremental), ); @@ -231,14 +232,13 @@ class LanguageServer { } }); - // The spec says we can return null here which I'd prefer to the empty list _connection.onDocumentLinks((params) async { try { var document = _documents.get(params.textDocument.uri); if (document == null) return []; var configuration = _getLanguageConfiguration(document); - if (configuration.links.enabled) { + if (configuration.documentLinks.enabled) { if (initialScan != null) { await initialScan; } @@ -254,6 +254,28 @@ class LanguageServer { } }); + _connection.onDocumentSymbol((params) async { + try { + var document = _documents.get(params.textDocument.uri); + if (document == null) return []; + + var configuration = _getLanguageConfiguration(document); + if (configuration.documentSymbols.enabled) { + if (initialScan != null) { + await initialScan; + } + + var result = _ls.findDocumentSymbols(document); + return Future.value(result.symbols); + } else { + return []; + } + } on Exception catch (e) { + _log.debug(e.toString()); + return []; + } + }); + _connection.onShutdown(() async { await _socket?.close(); exitCode = 0; 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 8870511..b3f6116 100644 --- a/pkgs/sass_language_services/lib/src/configuration/language_configuration.dart +++ b/pkgs/sass_language_services/lib/src/configuration/language_configuration.dart @@ -4,22 +4,22 @@ class FeatureConfiguration { FeatureConfiguration({required this.enabled}); } -class DefinitionConfiguration extends FeatureConfiguration { - DefinitionConfiguration({required super.enabled}); +class DocumentSymbolsConfiguration extends FeatureConfiguration { + DocumentSymbolsConfiguration({required super.enabled}); } -class LinksConfiguration extends FeatureConfiguration { - LinksConfiguration({required super.enabled}); +class DocumentLinksConfiguration extends FeatureConfiguration { + DocumentLinksConfiguration({required super.enabled}); } class LanguageConfiguration { - late final DefinitionConfiguration definition; - late final LinksConfiguration links; + late final DocumentSymbolsConfiguration documentSymbols; + late final DocumentLinksConfiguration documentLinks; LanguageConfiguration.from(dynamic config) { - definition = DefinitionConfiguration( - enabled: config?['definition']?['enabled'] as bool? ?? true); - links = LinksConfiguration( - enabled: config?['links']?['enabled'] as bool? ?? true); + documentSymbols = DocumentSymbolsConfiguration( + enabled: config?['documentSymbols']?['enabled'] as bool? ?? true); + documentLinks = DocumentLinksConfiguration( + enabled: config?['documentLinks']?['enabled'] as bool? ?? true); } } 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 4631b0a..0d5c534 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 @@ -9,7 +9,7 @@ class DocumentSymbolsFeature extends LanguageFeature { StylesheetDocumentSymbols findDocumentSymbols(TextDocument document) { var stylesheet = ls.parseStylesheet(document); - var symbolsVisitor = DocumentSymbolsVisitor(); + var symbolsVisitor = DocumentSymbolsVisitor(document); stylesheet.accept(symbolsVisitor); return symbolsVisitor.symbols; } 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 7e4fb63..ee53479 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 @@ -2,6 +2,7 @@ import 'package:lsp_server/lsp_server.dart' as lsp; import 'package:sass_api/sass_api.dart' as sass; import 'package:sass_language_services/src/utils/sass_lsp_utils.dart'; +import '../../lsp/text_document.dart'; import './stylesheet_document_symbols.dart'; import 'stylesheet_document_symbol.dart'; @@ -10,6 +11,10 @@ final quotes = RegExp('["\']'); class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { final symbols = StylesheetDocumentSymbols(); + final TextDocument _document; + + DocumentSymbolsVisitor(this._document); + @override void visitStyleRule(sass.StyleRule node) { super.visitStyleRule(node); @@ -25,8 +30,8 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { var symbol = StylesheetDocumentSymbol( name: complexSelector.span.text.trim(), kind: lsp.SymbolKind.Class, - range: toRange(complexSelector.span), - selectionRange: toRange(complexSelector.span)); + location: lsp.Location( + range: toRange(complexSelector.span), uri: _document.uri)); symbols.selectors.add(symbol); } diff --git a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart index ee84c52..32260c5 100644 --- a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart @@ -1,15 +1,12 @@ import 'package:lsp_server/lsp_server.dart'; -class StylesheetDocumentSymbol extends DocumentSymbol { +class StylesheetDocumentSymbol extends SymbolInformation { final String? documentation; StylesheetDocumentSymbol({ required super.name, required super.kind, - required super.range, - required super.selectionRange, - super.detail, - super.children, + required super.location, super.tags, super.deprecated, this.documentation, From 26a28ed176845146b44390be71751198c20618bc Mon Sep 17 00:00:00 2001 From: William Killerud Date: Fri, 8 Nov 2024 21:27:46 +0100 Subject: [PATCH 05/23] Link to roadmap issue, update docs about testing [skip ci] --- README.md | 13 ++++------- docs/contributing/development-environment.md | 4 +++- ...{debugging.md => testing-and-debugging.md} | 22 +++++++++---------- extension/test/README.md | 16 +++----------- 4 files changed, 21 insertions(+), 34 deletions(-) rename docs/contributing/{debugging.md => testing-and-debugging.md} (98%) diff --git a/README.md b/README.md index 8740d31..e645398 100644 --- a/README.md +++ b/README.md @@ -1,16 +1,11 @@ # Overview -This project is an early proof of concept of a language server for Sass written in Dart to make use of [sass_api](https://pub.dev/packages/sass_api). The proof of concept includes: +This is a work-in-progress language server for Sass written in Dart to +use [sass_api](https://pub.dev/packages/sass_api). -- Language server process with incremental document sync. -- User configuration. -- [Document links](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_documentLink) provider. -- Workspace scanner. -- VS Code client with debugging profile. - -Check the documentation to get started: +See the [initial version roadmap](https://github.com/sass/dart-sass-language-server/issues/2) for the state of different features or check the documentation to get started: - [Development environment](./docs/contributing/development-environment.md) -- [Debugging](./docs/contributing/debugging.md) +- [Testing and debugging](./docs/contributing/testing-and-debugging.md) - [Building](./docs/contributing/building.md) - [Architecture](./docs/contributing/architecture.md) diff --git a/docs/contributing/development-environment.md b/docs/contributing/development-environment.md index 8c15648..3c3aed1 100644 --- a/docs/contributing/development-environment.md +++ b/docs/contributing/development-environment.md @@ -1,6 +1,8 @@ ## Development environment -The Sass language server, like the Sass compiler, is written in [Dart](https://dart.dev/). The language extension for Visual Studio Code is written in [TypeScript](https://www.typescriptlang.org/). +The Sass language server, like the Sass compiler, is written in [Dart](https://dart.dev/). +The language extension for Visual Studio Code is written in [TypeScript](https://www.typescriptlang.org/). +Tests for the extension are written in JavaScript. If you have a background writing JavaScript or TypeScript, [Learning Dart as a JavaScript developer](https://dart.dev/resources/coming-from/js-to-dart) is a great place to start. diff --git a/docs/contributing/debugging.md b/docs/contributing/testing-and-debugging.md similarity index 98% rename from docs/contributing/debugging.md rename to docs/contributing/testing-and-debugging.md index 9ee4260..00b7930 100644 --- a/docs/contributing/debugging.md +++ b/docs/contributing/testing-and-debugging.md @@ -1,4 +1,4 @@ -# Debugging +# Testing and debugging Here we assume you have set up a [development environment](./development-environment.md). @@ -13,6 +13,16 @@ The quickest way to test the language server is to debug the language extension This will open another window of Visual Studio Code, this one running as an `[Extension Development Host]`. +### Testing in isolation + +VS Code ships with some built-in support for SCSS and CSS. To test this language server in isolation you can disable the built-in extension. + +1. Go to the Extensions tab and search for `@builtin css language features`. +2. Click the settings icon and pick Disable from the list. +3. Click Restart extension to turn it off. + +You should also turn off extensions like SCSS IntelliSense or Some Sass. + ### Open the Dart DevTools In this configuration, the client has run `dart run --observe` in the local `sass_language_server` package. You can now use [Dart DevTools](https://dart.dev/tools/dart-devtools) to debug the language server. @@ -61,13 +71,3 @@ test profile in the Run and Debug view in VS Code. ] } ``` - -## Testing in isolation - -VS Code ships with some built-in support for SCSS and CSS. To test this language server in isolation you can disable the built-in extension. - -1. Go to the Extensions tab and search for `@builtin css language features`. -2. Click the settings icon and pick Disable from the list. -3. Click Restart extension to turn it off. - -You should also turn off extensions like SCSS IntelliSense or Some Sass. diff --git a/extension/test/README.md b/extension/test/README.md index 51c2d77..1c743ed 100644 --- a/extension/test/README.md +++ b/extension/test/README.md @@ -3,23 +3,13 @@ These tests automate Visual Studio Code using the [VS Code JavaScript API](https://code.visualstudio.com/api/references/vscode-api). See [the list of built-in commands](https://code.visualstudio.com/api/references/commands#commands) to see how you can automate the interactions you need for testing. -There are two different test runners available: +The runner is configured so it can run multiple instances of VS Code (in sequence) using different configurations and workspaces. +Each subdirectory in `electron/` will run in a separate instance of VS Code. -- [@vscode/test-electron](https://github.com/microsoft/vscode-test) -- [@vscode/test-web](https://github.com/microsoft/vscode-test-web) - -We only test using Electron. - -The Electron runner is configured so it can run multiple instances of VS Code (in sequence) using -different configurations and workspaces. Each subdirectory in `electron/` will run in a separate instance -of VS Code. - -By convention subdirectories in `e2e/` must have: +By convention subdirectories in `electron/` must have: - `index.js` as the entrypoint that finds test files and passes them to Mocha - at least one `*.test.js` file with some tests - a `fixtures/` subdirectory with: - `.vscode/settings.json` - `styles.scss` or `styles.sass` - -Starting and stopping VS Code takes a few seconds, so try to use the same workspaces as much as possible. From eff7b7503beafa6b563a1653252dcf198632ea91 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Fri, 8 Nov 2024 21:40:38 +0100 Subject: [PATCH 06/23] Use raw strings for Sass code-as-string --- .../document_links/document_links_test.dart | 38 +++++++++---------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/pkgs/sass_language_services/test/features/document_links/document_links_test.dart b/pkgs/sass_language_services/test/features/document_links/document_links_test.dart index 6c01bd4..2b4ab16 100644 --- a/pkgs/sass_language_services/test/features/document_links/document_links_test.dart +++ b/pkgs/sass_language_services/test/features/document_links/document_links_test.dart @@ -14,15 +14,15 @@ void main() { }); test('should resolve valid links', () async { - fs.createDocument('\$var: 1px;', uri: 'variables.scss'); - fs.createDocument('\$tr: 2px;', uri: 'corners.scss'); - fs.createDocument('\$b: #000;', uri: 'colors.scss'); + fs.createDocument(r'$var: 1px;', uri: 'variables.scss'); + fs.createDocument(r'$tr: 2px;', uri: 'corners.scss'); + fs.createDocument(r'$b: #000;', uri: 'colors.scss'); - var document = fs.createDocument(''' + var document = fs.createDocument(r''' @use "corners" as *; @use "variables" as vars; -@forward "colors" as color-* hide \$foo, barfunc; -@forward "./does-not-exist" as foo-* show \$public; +@forward "colors" as color-* hide $foo, barfunc; +@forward "./does-not-exist" as foo-* show $public; '''); var links = await ls.findDocumentLinks(document); @@ -54,9 +54,9 @@ void main() { }); test('should resolve various relative links', () async { - fs.createDocument('\$var: 1px;', uri: 'upper.scss'); - fs.createDocument('\$tr: 2px;', uri: 'middle/middle.scss'); - fs.createDocument('\$b: #000;', uri: 'middle/lower/lower.scss'); + fs.createDocument(r'$var: 1px;', uri: 'upper.scss'); + fs.createDocument(r'$tr: 2px;', uri: 'middle/middle.scss'); + fs.createDocument(r'$b: #000;', uri: 'middle/lower/lower.scss'); var document = fs.createDocument(''' @use "../upper"; @@ -70,14 +70,14 @@ void main() { }); test('should not break on circular references', () async { - fs.createDocument(''' + fs.createDocument(r''' @use "./pong" -\$var: ping +$var: ping ''', uri: 'ping.sass'); - var document = fs.createDocument(''' + var document = fs.createDocument(r''' @use "./pong" -\$var: ping +$var: ping ''', uri: 'ping.sass'); var links = await ls.findDocumentLinks(document); @@ -86,12 +86,12 @@ void main() { }); test('handles various forms of partials', () async { - fs.createDocument(''' -\$foo: blue; + fs.createDocument(r''' +$foo: blue; ''', uri: '_foo.scss'); - fs.createDocument(''' -\$bar: red + fs.createDocument(r''' +$bar: red ''', uri: 'bar/_index.sass'); var document = fs.createDocument(''' @@ -129,8 +129,8 @@ void main() { }); test('handles Sass imports without string quotes', () async { - fs.createDocument(''' -\$foo: blue; + fs.createDocument(r''' +$foo: blue; ''', uri: '_foo.scss'); var links = await ls.findDocumentLinks(fs.createDocument(''' From 501f15437a0096a427307306673b7f5c93c6100e Mon Sep 17 00:00:00 2001 From: William Killerud Date: Fri, 8 Nov 2024 21:48:40 +0100 Subject: [PATCH 07/23] Variables, functions and mixins --- .../document_symbols_visitor.dart | 59 +++++++++++++++++++ .../document_symbols_test.dart | 24 ++++++++ 2 files changed, 83 insertions(+) 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 ee53479..c447f82 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 @@ -15,6 +15,53 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { DocumentSymbolsVisitor(this._document); + @override + void visitAtRule(node) { + super.visitAtRule(node); + // TODO: keyframe, fontface stuff? + } + + @override + void visitDeclaration(node) { + super.visitDeclaration(node); + if (node.name.isPlain && node.name.asPlain!.startsWith("--")) { + var symbol = StylesheetDocumentSymbol( + name: node.name.span.text.trim(), + kind: lsp.SymbolKind.Variable, + location: + lsp.Location(range: toRange(node.name.span), uri: _document.uri)); + symbols.cssVariables.add(symbol); + } + } + + @override + void visitFunctionRule(node) { + super.visitFunctionRule(node); + var symbol = StylesheetDocumentSymbol( + name: node.name, + kind: lsp.SymbolKind.Function, + location: + lsp.Location(range: toRange(node.nameSpan), uri: _document.uri)); + symbols.functions.add(symbol); + } + + @override + void visitMediaRule(node) { + super.visitMediaRule(node); + // TODO: media queries + } + + @override + void visitMixinRule(node) { + super.visitMixinRule(node); + var symbol = StylesheetDocumentSymbol( + name: node.name, + kind: lsp.SymbolKind.Function, + location: + lsp.Location(range: toRange(node.nameSpan), uri: _document.uri)); + symbols.mixins.add(symbol); + } + @override void visitStyleRule(sass.StyleRule node) { super.visitStyleRule(node); @@ -39,4 +86,16 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { // Do nothing. } } + + @override + void visitVariableDeclaration(node) { + super.visitVariableDeclaration(node); + var symbol = StylesheetDocumentSymbol( + // Include the $ since this field is user-facing + name: '\$${node.name}', + kind: lsp.SymbolKind.Variable, + location: + lsp.Location(range: toRange(node.nameSpan), uri: _document.uri)); + symbols.variables.add(symbol); + } } diff --git a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart index 8f9a4a4..f26a3bf 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart @@ -75,4 +75,28 @@ void main() { result.selectors.first.name, equals('.foo:has([data-testid="bar"])')); }); }); + + group('variables', () { + test('CSS variables', () { + var document = fs.createDocument(''' +.hello { + --world: blue; + color: var(--world); +} +'''); + var result = ls.findDocumentSymbols(document); + expect(result.cssVariables.first.name, equals('--world')); + }); + + test('public variables', () { + var document = fs.createDocument(r''' +$world: blue; +.hello { + color: $world; +} +'''); + var result = ls.findDocumentSymbols(document); + expect(result.variables.first.name, equals(r'$world')); + }); + }); } From 874364fcb0b0917bd229bd908b970c3bad821ae5 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sat, 9 Nov 2024 16:19:46 +0100 Subject: [PATCH 08/23] Clear cache between tests --- .../test/features/document_symbols/document_symbols_test.dart | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart index f26a3bf..27d4464 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart @@ -77,6 +77,10 @@ void main() { }); group('variables', () { + setUp(() { + ls.cache.clear(); + }); + test('CSS variables', () { var document = fs.createDocument(''' .hello { From 1028f2ba572a4a875f9cac770fc1b720a2cd5ee6 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sat, 9 Nov 2024 16:25:23 +0100 Subject: [PATCH 09/23] Test for function and mixin --- .../document_symbols_test.dart | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart index 27d4464..9316a47 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart @@ -103,4 +103,32 @@ $world: blue; expect(result.variables.first.name, equals(r'$world')); }); }); + + group('callables', () { + setUp(() { + ls.cache.clear(); + }); + + test('functions', () { + var document = fs.createDocument(r''' +@function doStuff($a: 1, $b: 2) { + $value: $a + $b; + @return $value; +} +'''); + var result = ls.findDocumentSymbols(document); + expect(result.functions.first.name, equals(r'doStuff')); + }); + + test('mixins', () { + var document = fs.createDocument(r''' +@mixin mixin1 { + $value: 1; + line-height: $value; +} +'''); + var result = ls.findDocumentSymbols(document); + expect(result.mixins.first.name, equals(r'mixin1')); + }); + }); } From d9264ac8fe7dc879fdb9ea887f138aa2aec49cf3 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sat, 9 Nov 2024 18:58:48 +0100 Subject: [PATCH 10/23] Font face and keyframes --- .../document_symbols_visitor.dart | 60 +++++++++++++++++-- .../stylesheet_document_symbol.dart | 4 +- .../document_symbols_test.dart | 42 +++++++++++++ 3 files changed, 99 insertions(+), 7 deletions(-) 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 c447f82..3fdf0f5 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 @@ -7,6 +7,7 @@ import './stylesheet_document_symbols.dart'; import 'stylesheet_document_symbol.dart'; final quotes = RegExp('["\']'); +final deprecated = RegExp(r'///\s*@deprecated'); class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { final symbols = StylesheetDocumentSymbols(); @@ -15,10 +16,46 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { DocumentSymbolsVisitor(this._document); + bool _isDeprecated(String? docComment) { + if (docComment != null && deprecated.hasMatch(docComment)) { + return true; + } + return false; + } + @override void visitAtRule(node) { super.visitAtRule(node); - // TODO: keyframe, fontface stuff? + if (!node.name.isPlain) { + return; + } + + if (node.name.asPlain == 'font-face') { + var symbol = StylesheetDocumentSymbol( + name: '@${node.name.span.text.trim()}', + kind: lsp.SymbolKind.Class, + location: + lsp.Location(range: toRange(node.name.span), uri: _document.uri)); + symbols.fontFaces.add(symbol); + } else if (node.name.asPlain!.startsWith('keyframes')) { + var keyframesName = node.span.context.split(' ').elementAtOrNull(1); + if (keyframesName != null) { + var keyframesNameRange = lsp.Range( + start: lsp.Position( + line: node.name.span.start.line, + character: node.span.end.column + 1), + end: lsp.Position( + line: node.span.end.line, + character: node.span.end.column + 1 + keyframesName.length)); + + var symbol = StylesheetDocumentSymbol( + name: keyframesName, + kind: lsp.SymbolKind.Class, + location: + lsp.Location(range: keyframesNameRange, uri: _document.uri)); + symbols.keyframeIdentifiers.add(symbol); + } + } } @override @@ -41,14 +78,23 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { name: node.name, kind: lsp.SymbolKind.Function, location: - lsp.Location(range: toRange(node.nameSpan), uri: _document.uri)); + lsp.Location(range: toRange(node.nameSpan), uri: _document.uri), + docComment: node.comment?.docComment, + deprecated: _isDeprecated(node.comment?.docComment)); symbols.functions.add(symbol); } @override void visitMediaRule(node) { super.visitMediaRule(node); - // TODO: media queries + if (!node.query.isPlain) { + return; + } + var symbol = StylesheetDocumentSymbol( + name: '@media ${node.query.asPlain}', + kind: lsp.SymbolKind.Module, + location: lsp.Location(range: toRange(node.span), uri: _document.uri)); + symbols.mediaQueries.add(symbol); } @override @@ -58,7 +104,9 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { name: node.name, kind: lsp.SymbolKind.Function, location: - lsp.Location(range: toRange(node.nameSpan), uri: _document.uri)); + lsp.Location(range: toRange(node.nameSpan), uri: _document.uri), + docComment: node.comment?.docComment, + deprecated: _isDeprecated(node.comment?.docComment)); symbols.mixins.add(symbol); } @@ -95,7 +143,9 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { name: '\$${node.name}', kind: lsp.SymbolKind.Variable, location: - lsp.Location(range: toRange(node.nameSpan), uri: _document.uri)); + lsp.Location(range: toRange(node.nameSpan), uri: _document.uri), + docComment: node.comment?.docComment, + deprecated: _isDeprecated(node.comment?.docComment)); symbols.variables.add(symbol); } } diff --git a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart index 32260c5..184c1c8 100644 --- a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart @@ -1,7 +1,7 @@ import 'package:lsp_server/lsp_server.dart'; class StylesheetDocumentSymbol extends SymbolInformation { - final String? documentation; + final String? docComment; StylesheetDocumentSymbol({ required super.name, @@ -9,6 +9,6 @@ class StylesheetDocumentSymbol extends SymbolInformation { required super.location, super.tags, super.deprecated, - this.documentation, + this.docComment, }); } diff --git a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart index 9316a47..3c05aa3 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart @@ -118,6 +118,9 @@ $world: blue; '''); var result = ls.findDocumentSymbols(document); expect(result.functions.first.name, equals(r'doStuff')); + // Don't include function arguments + expect(result.variables.length, equals(1)); + expect(result.variables.last.name, equals(r'$value')); }); test('mixins', () { @@ -129,6 +132,45 @@ $world: blue; '''); var result = ls.findDocumentSymbols(document); expect(result.mixins.first.name, equals(r'mixin1')); + expect(result.variables.first.name, equals(r'$value')); + }); + }); + + group('at-rules', () { + setUp(() { + ls.cache.clear(); + }); + + test('@media', () { + var document = fs.createDocument(r''' +@media screen, print { + body { + font-size: 14pt; + } +} +'''); + var result = ls.findDocumentSymbols(document); + expect(result.mediaQueries.first.name, equals('@media screen, print')); + }); + + test('@font-face', () { + var document = fs.createDocument(r''' +@font-face { + font-family: "Vulf Mono", monospace; +} +'''); + var result = ls.findDocumentSymbols(document); + expect(result.fontFaces.first.name, equals('@font-face')); + }); + + test('@keyframes', () { + var document = fs.createDocument(r''' +@keyframes animation { + +} +'''); + var result = ls.findDocumentSymbols(document); + expect(result.keyframeIdentifiers.first.name, equals('animation')); }); }); } From cbcee8a3fb8c3c44a4338cc58f744c9f7a4657b3 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sat, 9 Nov 2024 21:53:09 +0100 Subject: [PATCH 11/23] Refactor - Use the recommended DocumentSymbol return type. - Said DocumentSymbol should include a hierarchy. --- .../document-links/fixtures/styles.scss | 6 + .../lib/src/language_server.dart | 18 +- .../lib/sass_language_services.dart | 1 + .../document_symbols_feature.dart | 7 +- .../document_symbols_visitor.dart | 214 ++++++++++++++---- .../stylesheet_document_symbol.dart | 7 +- .../stylesheet_document_symbols.dart | 43 ---- .../lib/src/language_services.dart | 5 +- .../document_symbols_test.dart | 66 ++++-- 9 files changed, 245 insertions(+), 122 deletions(-) delete mode 100644 pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart diff --git a/extension/test/electron/document-links/fixtures/styles.scss b/extension/test/electron/document-links/fixtures/styles.scss index 0d32c36..1d54d4c 100644 --- a/extension/test/electron/document-links/fixtures/styles.scss +++ b/extension/test/electron/document-links/fixtures/styles.scss @@ -4,3 +4,9 @@ button { color: colors.$color-primary; } + +@media screen, print { + body { + font-size: 14pt; + } +} diff --git a/pkgs/sass_language_server/lib/src/language_server.dart b/pkgs/sass_language_server/lib/src/language_server.dart index 614f265..20904d9 100644 --- a/pkgs/sass_language_server/lib/src/language_server.dart +++ b/pkgs/sass_language_server/lib/src/language_server.dart @@ -254,9 +254,13 @@ class LanguageServer { } }); - _connection.onDocumentSymbol((params) async { + // TODO: upstream allowing DocumentSymbol here + Future> onDocumentSymbol(dynamic params) async { try { - var document = _documents.get(params.textDocument.uri); + var documentSymbolParams = DocumentSymbolParams.fromJson( + params.value as Map); + + var document = _documents.get(documentSymbolParams.textDocument.uri); if (document == null) return []; var configuration = _getLanguageConfiguration(document); @@ -266,7 +270,7 @@ class LanguageServer { } var result = _ls.findDocumentSymbols(document); - return Future.value(result.symbols); + return Future.value(result); } else { return []; } @@ -274,7 +278,10 @@ class LanguageServer { _log.debug(e.toString()); return []; } - }); + } + + _connection.peer + .registerMethod('textDocument/documentSymbol', onDocumentSymbol); _connection.onShutdown(() async { await _socket?.close(); @@ -286,7 +293,8 @@ class LanguageServer { }); _connection.listen(); - } on SocketException catch (_) { + } on Exception catch (e) { + _log.error(e.toString()); exit(exitCode); } } diff --git a/pkgs/sass_language_services/lib/sass_language_services.dart b/pkgs/sass_language_services/lib/sass_language_services.dart index 9c0d854..6645e70 100644 --- a/pkgs/sass_language_services/lib/sass_language_services.dart +++ b/pkgs/sass_language_services/lib/sass_language_services.dart @@ -21,3 +21,4 @@ export 'src/file_system_provider.dart' export 'src/language_services.dart' show LanguageServices; export 'src/features/document_links/stylesheet_document_link.dart'; +export 'src/features/document_symbols/stylesheet_document_symbol.dart'; 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 0d5c534..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 @@ -2,15 +2,16 @@ import 'package:sass_language_services/sass_language_services.dart'; import '../language_feature.dart'; import 'document_symbols_visitor.dart'; -import 'stylesheet_document_symbols.dart'; class DocumentSymbolsFeature extends LanguageFeature { DocumentSymbolsFeature({required super.ls}); - StylesheetDocumentSymbols findDocumentSymbols(TextDocument document) { + List findDocumentSymbols(TextDocument document) { var stylesheet = ls.parseStylesheet(document); - var symbolsVisitor = DocumentSymbolsVisitor(document); + + var symbolsVisitor = DocumentSymbolsVisitor(); stylesheet.accept(symbolsVisitor); + return symbolsVisitor.symbols; } } 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 3fdf0f5..e06e7b4 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 @@ -2,19 +2,86 @@ import 'package:lsp_server/lsp_server.dart' as lsp; import 'package:sass_api/sass_api.dart' as sass; import 'package:sass_language_services/src/utils/sass_lsp_utils.dart'; -import '../../lsp/text_document.dart'; -import './stylesheet_document_symbols.dart'; import 'stylesheet_document_symbol.dart'; final quotes = RegExp('["\']'); final deprecated = RegExp(r'///\s*@deprecated'); class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { - final symbols = StylesheetDocumentSymbols(); + final symbols = []; - final TextDocument _document; + DocumentSymbolsVisitor(); - DocumentSymbolsVisitor(this._document); + /// Sort out the relationship between parent and child symbols. + /// + /// The visitor begins with the leaf nodes and travels recursively + /// up before moving on to the next branch. We check to see + /// if the symbol we're collecting now contains (based on its range) + /// other symbols, and if so adds them as children. + void _collect( + {required String name, + required lsp.SymbolKind kind, + required lsp.Range symbolRange, + lsp.Range? nameRange, + lsp.Range? bodyRange, // TODO: delete if unused + String? docComment, + String? detail, + List? tags}) { + var range = symbolRange; + var selectionRange = nameRange; + if (selectionRange == null || !_containsRange(range, symbolRange)) { + selectionRange = lsp.Range(start: range.start, end: range.end); + } + + var symbol = StylesheetDocumentSymbol( + name: name, + kind: kind, + range: range, + children: [], + selectionRange: selectionRange, + docComment: docComment, + detail: detail, + tags: tags, + deprecated: _isDeprecated(docComment)); + + // Look to see if this symbol contains other symbols. + + var keep = []; + + for (var other in symbols) { + // This probably scales terribly with document size. + // A tree structure that uses ranges for lookup would probably be the ticket here. + // We can maybe get away with it if we cache the result. + if (_containsRange(symbol.range, other.range)) { + symbol.children!.add(other); + } else { + keep.add(other); + } + } + + keep.add(symbol); + symbols.replaceRange(0, symbols.length, keep); + } + + bool _containsRange(lsp.Range a, lsp.Range b) { + if (b.start.line < a.start.line || b.end.line < a.start.line) { + return false; + } + + if (b.start.line > b.end.line || b.end.line > a.end.line) { + return false; + } + + if (b.start.line == a.start.line && b.start.character < a.start.character) { + return false; + } + + if (b.end.line == a.end.line && b.end.character > a.end.character) { + return false; + } + + return true; + } bool _isDeprecated(String? docComment) { if (docComment != null && deprecated.hasMatch(docComment)) { @@ -23,6 +90,33 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { return false; } + lsp.Range? _bodyRange(sass.ParentStatement?> node) { + if (node.children case var children?) { + if (children.isEmpty) { + return null; + } + + return lsp.Range( + start: lsp.Position( + line: children.first.span.start.line, + character: children.first.span.start.column), + end: lsp.Position( + line: children.last.span.start.line, + character: children.last.span.start.column)); + } + + return null; + } + + String? _detail(sass.CallableDeclaration node) { + var arguments = node.arguments.arguments + .map((arg) => arg.defaultValue != null + ? '${arg.name}: ${arg.defaultValue!.span.text}' + : arg.name) + .join(', '); + return '($arguments)'; + } + @override void visitAtRule(node) { super.visitAtRule(node); @@ -31,12 +125,12 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { } if (node.name.asPlain == 'font-face') { - var symbol = StylesheetDocumentSymbol( - name: '@${node.name.span.text.trim()}', + _collect( + name: node.name.span.text, kind: lsp.SymbolKind.Class, - location: - lsp.Location(range: toRange(node.name.span), uri: _document.uri)); - symbols.fontFaces.add(symbol); + symbolRange: toRange(node.span), + nameRange: toRange(node.name.span), + bodyRange: _bodyRange(node)); } else if (node.name.asPlain!.startsWith('keyframes')) { var keyframesName = node.span.context.split(' ').elementAtOrNull(1); if (keyframesName != null) { @@ -48,12 +142,12 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { line: node.span.end.line, character: node.span.end.column + 1 + keyframesName.length)); - var symbol = StylesheetDocumentSymbol( + _collect( name: keyframesName, kind: lsp.SymbolKind.Class, - location: - lsp.Location(range: keyframesNameRange, uri: _document.uri)); - symbols.keyframeIdentifiers.add(symbol); + symbolRange: toRange(node.span), + nameRange: keyframesNameRange, + bodyRange: _bodyRange(node)); } } } @@ -62,26 +156,27 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { void visitDeclaration(node) { super.visitDeclaration(node); if (node.name.isPlain && node.name.asPlain!.startsWith("--")) { - var symbol = StylesheetDocumentSymbol( - name: node.name.span.text.trim(), + _collect( + name: node.name.span.text, kind: lsp.SymbolKind.Variable, - location: - lsp.Location(range: toRange(node.name.span), uri: _document.uri)); - symbols.cssVariables.add(symbol); + symbolRange: toRange(node.span), + nameRange: toRange(node.name.span), + bodyRange: _bodyRange(node)); } } @override void visitFunctionRule(node) { super.visitFunctionRule(node); - var symbol = StylesheetDocumentSymbol( + + _collect( name: node.name, + detail: _detail(node), kind: lsp.SymbolKind.Function, - location: - lsp.Location(range: toRange(node.nameSpan), uri: _document.uri), docComment: node.comment?.docComment, - deprecated: _isDeprecated(node.comment?.docComment)); - symbols.functions.add(symbol); + symbolRange: toRange(node.span), + nameRange: toRange(node.nameSpan), + bodyRange: _bodyRange(node)); } @override @@ -90,24 +185,27 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { if (!node.query.isPlain) { return; } - var symbol = StylesheetDocumentSymbol( + + _collect( name: '@media ${node.query.asPlain}', kind: lsp.SymbolKind.Module, - location: lsp.Location(range: toRange(node.span), uri: _document.uri)); - symbols.mediaQueries.add(symbol); + symbolRange: toRange(node.span), + nameRange: toRange(node.query.span), + bodyRange: _bodyRange(node)); } @override void visitMixinRule(node) { super.visitMixinRule(node); - var symbol = StylesheetDocumentSymbol( + + _collect( name: node.name, - kind: lsp.SymbolKind.Function, - location: - lsp.Location(range: toRange(node.nameSpan), uri: _document.uri), + detail: _detail(node), + kind: lsp.SymbolKind.Method, docComment: node.comment?.docComment, - deprecated: _isDeprecated(node.comment?.docComment)); - symbols.mixins.add(symbol); + symbolRange: toRange(node.span), + nameRange: toRange(node.nameSpan), + bodyRange: _bodyRange(node)); } @override @@ -122,13 +220,36 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { try { var selectorList = sass.SelectorList.parse(node.selector.asPlain!); for (var complexSelector in selectorList.components) { - var symbol = StylesheetDocumentSymbol( - name: complexSelector.span.text.trim(), - kind: lsp.SymbolKind.Class, - location: lsp.Location( - range: toRange(complexSelector.span), uri: _document.uri)); + var nameRange = toRange(complexSelector.span); + var nameWithoutTrailingSpace = complexSelector.span.text.trimRight(); + var diff = + complexSelector.span.text.length - nameWithoutTrailingSpace.length; + if (diff != 0) { + nameRange = lsp.Range( + start: lsp.Position( + line: node.span.start.line + nameRange.start.line, + character: + node.span.start.column + nameRange.start.character), + end: lsp.Position( + line: node.span.start.line + nameRange.end.line, + character: + node.span.start.column + nameRange.end.character - diff)); + } + + // symbolRange: start position of selector's nameRange, end of stylerule (node.span.end). + var symbolRange = lsp.Range( + start: lsp.Position( + line: nameRange.start.line, + character: nameRange.start.character), + end: lsp.Position( + line: node.span.end.line, character: node.span.end.column)); - symbols.selectors.add(symbol); + _collect( + name: nameWithoutTrailingSpace, + kind: lsp.SymbolKind.Class, + symbolRange: symbolRange, + nameRange: nameRange, + bodyRange: _bodyRange(node)); } } on sass.SassFormatException catch (_) { // Do nothing. @@ -138,14 +259,19 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { @override void visitVariableDeclaration(node) { super.visitVariableDeclaration(node); - var symbol = StylesheetDocumentSymbol( + _collect( // Include the $ since this field is user-facing name: '\$${node.name}', kind: lsp.SymbolKind.Variable, - location: - lsp.Location(range: toRange(node.nameSpan), uri: _document.uri), docComment: node.comment?.docComment, - deprecated: _isDeprecated(node.comment?.docComment)); - symbols.variables.add(symbol); + symbolRange: toRange(node.span), + nameRange: lsp.Range( + start: lsp.Position( + line: node.nameSpan.start.line, + // Include the $ in the range + character: node.nameSpan.start.column - 1), + end: lsp.Position( + line: node.nameSpan.end.line, + character: node.nameSpan.end.column))); } } diff --git a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart index 184c1c8..1af898e 100644 --- a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart +++ b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbol.dart @@ -1,14 +1,17 @@ import 'package:lsp_server/lsp_server.dart'; -class StylesheetDocumentSymbol extends SymbolInformation { +class StylesheetDocumentSymbol extends DocumentSymbol { final String? docComment; StylesheetDocumentSymbol({ required super.name, required super.kind, - required super.location, + required super.range, + required super.selectionRange, super.tags, super.deprecated, + super.detail, + super.children, this.docComment, }); } diff --git a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart deleted file mode 100644 index a0dec7c..0000000 --- a/pkgs/sass_language_services/lib/src/features/document_symbols/stylesheet_document_symbols.dart +++ /dev/null @@ -1,43 +0,0 @@ -import 'package:sass_language_services/src/features/document_symbols/stylesheet_document_symbol.dart'; - -class StylesheetDocumentSymbols { - /// Sass variable declarations. - final variables = []; - - /// Sass function declarations. - final functions = []; - - /// Sass mixin declarations. - final mixins = []; - - /// Placeholder selectors this document declares. - final placeholders = []; - - /// Placeholder selectors this document @extends. - final placeholderUsages = []; - - /// Declared CSS selectors. - final selectors = []; - - /// Declared CSS variables. - final cssVariables = []; - - final keyframeIdentifiers = []; - - final fontFaces = []; - - final mediaQueries = []; - - List get symbols => [ - ...variables, - ...functions, - ...mixins, - ...placeholders, - ...placeholderUsages, - ...selectors, - ...cssVariables, - ...keyframeIdentifiers, - ...fontFaces, - ...mediaQueries - ]; -} diff --git a/pkgs/sass_language_services/lib/src/language_services.dart b/pkgs/sass_language_services/lib/src/language_services.dart index 454a47e..515f3de 100644 --- a/pkgs/sass_language_services/lib/src/language_services.dart +++ b/pkgs/sass_language_services/lib/src/language_services.dart @@ -2,9 +2,8 @@ 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 'features/document_symbols/document_symbols_feature.dart'; -import 'features/document_symbols/stylesheet_document_symbols.dart'; import 'features/document_links/document_links_feature.dart'; +import 'features/document_symbols/document_symbols_feature.dart'; import 'language_services_cache.dart'; class LanguageServices { @@ -35,7 +34,7 @@ class LanguageServices { return _documentLinks.findDocumentLinks(document); } - StylesheetDocumentSymbols findDocumentSymbols(TextDocument document) { + List findDocumentSymbols(TextDocument document) { return _documentSymbols.findDocumentSymbols(document); } diff --git a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart index 3c05aa3..a4852e9 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/document_symbols_test.dart @@ -25,10 +25,10 @@ void main() { '''); var result = ls.findDocumentSymbols(document); - expect(result.symbols.length, equals(2)); + expect(result.length, equals(2)); - expect(result.selectors.first.name, equals(".foo")); - expect(result.selectors.last.name, equals(".bar")); + expect(result.first.name, equals(".foo")); + expect(result.last.name, equals(".bar")); }); test('should treat CSS selectors with multiple classes as one', () { @@ -43,10 +43,10 @@ void main() { '''); var result = ls.findDocumentSymbols(document); - expect(result.symbols.length, equals(2)); + expect(result.length, equals(2)); - expect(result.selectors.first.name, equals(".foo.bar")); - expect(result.selectors.last.name, equals(".fizz .buzz")); + expect(result.first.name, equals(".foo.bar")); + expect(result.last.name, equals(".fizz .buzz")); }); test('should treat lists of selectors as separate', () { @@ -58,10 +58,10 @@ void main() { '''); var result = ls.findDocumentSymbols(document); - expect(result.symbols.length, equals(2)); + expect(result.length, equals(2)); - expect(result.selectors.first.name, equals(".foo.bar")); - expect(result.selectors.last.name, equals(".fizz .buzz")); + expect(result.first.name, equals(".foo.bar")); + expect(result.last.name, equals(".fizz .buzz")); }); test('should include extras', () { @@ -71,8 +71,17 @@ void main() { } '''); var result = ls.findDocumentSymbols(document); - expect( - result.selectors.first.name, equals('.foo:has([data-testid="bar"])')); + expect(result.first.name, equals('.foo:has([data-testid="bar"])')); + }); + + test('placeholder selectors', () { + var document = fs.createDocument(''' +%waitforit { + color: red; +} +'''); + var result = ls.findDocumentSymbols(document); + expect(result.first.name, equals('%waitforit')); }); }); @@ -89,7 +98,11 @@ void main() { } '''); var result = ls.findDocumentSymbols(document); - expect(result.cssVariables.first.name, equals('--world')); + expect(result.length, equals(1)); + expect(result.first.name, equals('.hello')); + + expect(result.first.children!.length, equals(1)); + expect(result.first.children!.first.name, equals('--world')); }); test('public variables', () { @@ -100,7 +113,7 @@ $world: blue; } '''); var result = ls.findDocumentSymbols(document); - expect(result.variables.first.name, equals(r'$world')); + expect(result.first.name, equals(r'$world')); }); }); @@ -117,10 +130,11 @@ $world: blue; } '''); var result = ls.findDocumentSymbols(document); - expect(result.functions.first.name, equals(r'doStuff')); - // Don't include function arguments - expect(result.variables.length, equals(1)); - expect(result.variables.last.name, equals(r'$value')); + expect(result.length, equals(1)); + expect(result.first.name, equals('doStuff')); + + expect(result.first.children!.length, equals(1)); + expect(result.first.children!.first.name, equals(r'$value')); }); test('mixins', () { @@ -131,8 +145,12 @@ $world: blue; } '''); var result = ls.findDocumentSymbols(document); - expect(result.mixins.first.name, equals(r'mixin1')); - expect(result.variables.first.name, equals(r'$value')); + + expect(result.length, equals(1)); + expect(result.first.name, equals(r'mixin1')); + + expect(result.first.children!.length, equals(1)); + expect(result.first.children!.first.name, equals(r'$value')); }); }); @@ -150,7 +168,11 @@ $world: blue; } '''); var result = ls.findDocumentSymbols(document); - expect(result.mediaQueries.first.name, equals('@media screen, print')); + expect(result.length, equals(1)); + expect(result.first.name, equals('@media screen, print')); + + expect(result.first.children!.length, equals(1)); + expect(result.first.children!.first.name, equals('body')); }); test('@font-face', () { @@ -160,7 +182,7 @@ $world: blue; } '''); var result = ls.findDocumentSymbols(document); - expect(result.fontFaces.first.name, equals('@font-face')); + expect(result.first.name, equals('font-face')); }); test('@keyframes', () { @@ -170,7 +192,7 @@ $world: blue; } '''); var result = ls.findDocumentSymbols(document); - expect(result.keyframeIdentifiers.first.name, equals('animation')); + expect(result.first.name, equals('animation')); }); }); } From d9ffb3ad42b44d0de959fe9c35efc264265e34b6 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 11:04:28 +0100 Subject: [PATCH 12/23] Range tests --- .../document-symbols/document-symbols.test.js | 5 +- .../document-symbols/fixtures/styles.sass | 4 +- .../docyment_symbol_ranges_test.dart | 65 +++++++++++++++++++ .../test/position_matchers.dart | 40 ++++++++++++ 4 files changed, 109 insertions(+), 5 deletions(-) create mode 100644 pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart create mode 100644 pkgs/sass_language_services/test/position_matchers.dart diff --git a/extension/test/electron/document-symbols/document-symbols.test.js b/extension/test/electron/document-symbols/document-symbols.test.js index 067b6d1..f324d52 100644 --- a/extension/test/electron/document-symbols/document-symbols.test.js +++ b/extension/test/electron/document-symbols/document-symbols.test.js @@ -21,15 +21,14 @@ after(async () => { * @returns {Promise} */ async function findDocumentSymbols(documentUri) { - const links = await vscode.commands.executeCommand( + const result = await vscode.commands.executeCommand( 'vscode.executeDocumentSymbolProvider', documentUri ); - return links; + return result; } test('gets CSS selectors', async () => { - await showFile(stylesUri); const result = await findDocumentSymbols(stylesUri); assert.ok( diff --git a/extension/test/electron/document-symbols/fixtures/styles.sass b/extension/test/electron/document-symbols/fixtures/styles.sass index d684949..28b152f 100644 --- a/extension/test/electron/document-symbols/fixtures/styles.sass +++ b/extension/test/electron/document-symbols/fixtures/styles.sass @@ -7,8 +7,8 @@ button .world color: blue -.card > .header -.card > .body +.card > .header, +.card > .body, .card > .footer padding: 2px diff --git a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart new file mode 100644 index 0000000..87a816f --- /dev/null +++ b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart @@ -0,0 +1,65 @@ +import 'package:sass_language_services/sass_language_services.dart'; +import 'package:test/test.dart'; + +import '../../memory_file_system.dart'; +import '../../position_matchers.dart'; +import '../../test_client_capabilities.dart'; + +final fs = MemoryFileSystem(); +final ls = LanguageServices(fs: fs, clientCapabilities: getCapabilities()); + +void main() { + group('selectors', () { + setUp(() { + ls.cache.clear(); + }); + + test('CSS selector ranges are correct', () { + var document = fs.createDocument(''' +.foo { + color: red; +} +'''); + + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.selectionRange; + var symbolRange = result.first.range; + + expect(nameRange.start, AtLine(0)); + expect(nameRange.start, AtCharacter(0)); + + expect(nameRange.end, AtLine(0)); + expect(nameRange.end, AtCharacter(4)); + + expect(symbolRange.start, AtLine(0)); + expect(symbolRange.start, AtCharacter(0)); + + expect(symbolRange.end, AtLine(2)); + expect(symbolRange.end, AtCharacter(1)); + }); + + test('Sass indented selector ranges are correct', () { + var document = fs.createDocument(''' +.foo + color: red + +''', uri: 'index.sass'); + + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.selectionRange; + var symbolRange = result.first.range; + + expect(nameRange, StartsAtLine(0)); + expect(nameRange, StartsAtCharacter(0)); + + expect(nameRange, EndsAtLine(0)); + expect(nameRange, EndsAtCharacter(4)); + + expect(symbolRange, StartsAtLine(0)); + expect(symbolRange, StartsAtCharacter(0)); + + expect(symbolRange, EndsAtLine(2)); + expect(symbolRange, EndsAtCharacter(1)); + }); + }); +} diff --git a/pkgs/sass_language_services/test/position_matchers.dart b/pkgs/sass_language_services/test/position_matchers.dart new file mode 100644 index 0000000..967df2b --- /dev/null +++ b/pkgs/sass_language_services/test/position_matchers.dart @@ -0,0 +1,40 @@ +import 'package:lsp_server/lsp_server.dart'; +import 'package:test/test.dart'; + +class AtLine extends CustomMatcher { + AtLine(Object? valueOrMatcher) + : super('Position at line', 'line', valueOrMatcher); + + featureValueOf(actual) => (actual as Position).line; +} + +class AtCharacter extends CustomMatcher { + AtCharacter(Object? valueOrMatcher) + : super('Position at character', 'character', valueOrMatcher); + + featureValueOf(actual) => (actual as Position).character; +} + +class StartsAtLine extends CustomMatcher { + StartsAtLine(Object? valueOrMatcher) + : super('Range starts at line', 'line', valueOrMatcher); + featureValueOf(actual) => (actual as Range).start.line; +} + +class StartsAtCharacter extends CustomMatcher { + StartsAtCharacter(Object? valueOrMatcher) + : super('Range starts at character', 'character', valueOrMatcher); + featureValueOf(actual) => (actual as Range).start.character; +} + +class EndsAtLine extends CustomMatcher { + EndsAtLine(Object? valueOrMatcher) + : super('Range ends at line', 'line', valueOrMatcher); + featureValueOf(actual) => (actual as Range).end.line; +} + +class EndsAtCharacter extends CustomMatcher { + EndsAtCharacter(Object? valueOrMatcher) + : super('Range ends at character', 'character', valueOrMatcher); + featureValueOf(actual) => (actual as Range).end.character; +} From 8c5c74adf6ad3cd1d901e3e364ad986114fade3c Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 11:25:20 +0100 Subject: [PATCH 13/23] Fix ranges for Sass CSS selectors --- .../document_symbols_visitor.dart | 67 ++++++++++++------- .../docyment_symbol_ranges_test.dart | 4 +- 2 files changed, 45 insertions(+), 26 deletions(-) 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 e06e7b4..8ce9baa 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 @@ -220,34 +220,53 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { try { var selectorList = sass.SelectorList.parse(node.selector.asPlain!); for (var complexSelector in selectorList.components) { - var nameRange = toRange(complexSelector.span); - var nameWithoutTrailingSpace = complexSelector.span.text.trimRight(); - var diff = - complexSelector.span.text.length - nameWithoutTrailingSpace.length; - if (diff != 0) { - nameRange = lsp.Range( - start: lsp.Position( - line: node.span.start.line + nameRange.start.line, - character: - node.span.start.column + nameRange.start.character), - end: lsp.Position( - line: node.span.start.line + nameRange.end.line, - character: - node.span.start.column + nameRange.end.character - diff)); + String? name; + lsp.Range? nameRange; + lsp.Range? symbolRange; + + for (var component in complexSelector.components) { + var selector = component.selector; + + if (name == null) { + name = selector.span.text; + } else { + name = '$name ${selector.span.text}'; + } + + if (nameRange == null) { + // The selector span seems to be relative to node, not to the file. + nameRange = lsp.Range( + start: lsp.Position( + line: node.span.start.line + selector.span.start.line, + character: + node.span.start.column + selector.span.start.column), + end: lsp.Position( + line: node.span.start.line + selector.span.end.line, + character: + node.span.start.column + selector.span.end.column)); + + // symbolRange: start position of selector's nameRange, end of stylerule (node.span.end). + symbolRange = lsp.Range( + start: lsp.Position( + line: nameRange.start.line, + character: nameRange.start.character), + end: lsp.Position( + line: node.span.end.line, character: node.span.end.column)); + } else { + // Move the end of the name range down to include this selector component + 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)); + } } - // symbolRange: start position of selector's nameRange, end of stylerule (node.span.end). - var symbolRange = lsp.Range( - start: lsp.Position( - line: nameRange.start.line, - character: nameRange.start.character), - end: lsp.Position( - line: node.span.end.line, character: node.span.end.column)); - _collect( - name: nameWithoutTrailingSpace, + name: name!, kind: lsp.SymbolKind.Class, - symbolRange: symbolRange, + symbolRange: symbolRange!, nameRange: nameRange, bodyRange: _bodyRange(node)); } diff --git a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart index 87a816f..7abf93d 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart @@ -58,8 +58,8 @@ void main() { expect(symbolRange, StartsAtLine(0)); expect(symbolRange, StartsAtCharacter(0)); - expect(symbolRange, EndsAtLine(2)); - expect(symbolRange, EndsAtCharacter(1)); + expect(symbolRange, EndsAtLine(1)); + expect(symbolRange, EndsAtCharacter(12)); }); }); } From 3c280d7577e20fad50d6a791cc57d60580ce2ba9 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 11:30:15 +0100 Subject: [PATCH 14/23] Test two selector ranges in same document --- .../docyment_symbol_ranges_test.dart | 38 ++++++++++++++----- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart index 7abf93d..8d37138 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart @@ -19,23 +19,41 @@ void main() { .foo { color: red; } + +.bar { + color: blue; +} '''); var result = ls.findDocumentSymbols(document); - var nameRange = result.first.selectionRange; - var symbolRange = result.first.range; + var fooNameRange = result.first.selectionRange; + var fooSymbolRange = result.first.range; + expect(fooNameRange.start, AtLine(0)); + expect(fooNameRange.start, AtCharacter(0)); + + expect(fooNameRange.end, AtLine(0)); + expect(fooNameRange.end, AtCharacter(4)); + + expect(fooSymbolRange.start, AtLine(0)); + expect(fooSymbolRange.start, AtCharacter(0)); + + expect(fooSymbolRange.end, AtLine(2)); + expect(fooSymbolRange.end, AtCharacter(1)); + + var barNameRange = result.last.selectionRange; + var barSymbolRange = result.last.range; - expect(nameRange.start, AtLine(0)); - expect(nameRange.start, AtCharacter(0)); + expect(barNameRange.start, AtLine(4)); + expect(barNameRange.start, AtCharacter(0)); - expect(nameRange.end, AtLine(0)); - expect(nameRange.end, AtCharacter(4)); + expect(barNameRange.end, AtLine(4)); + expect(barNameRange.end, AtCharacter(4)); - expect(symbolRange.start, AtLine(0)); - expect(symbolRange.start, AtCharacter(0)); + expect(barSymbolRange.start, AtLine(4)); + expect(barSymbolRange.start, AtCharacter(0)); - expect(symbolRange.end, AtLine(2)); - expect(symbolRange.end, AtCharacter(1)); + expect(barSymbolRange.end, AtLine(6)); + expect(barSymbolRange.end, AtCharacter(1)); }); test('Sass indented selector ranges are correct', () { From 713767d056af70fa5662464016cff3911948dfdb Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 11:44:06 +0100 Subject: [PATCH 15/23] Fix variable range --- .../document_symbols_visitor.dart | 7 +- .../docyment_symbol_ranges_test.dart | 110 ++++++++++++++++++ 2 files changed, 113 insertions(+), 4 deletions(-) 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 8ce9baa..3291ec5 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 @@ -279,16 +279,15 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { void visitVariableDeclaration(node) { super.visitVariableDeclaration(node); _collect( - // Include the $ since this field is user-facing - name: '\$${node.name}', + name: node.nameSpan.text, kind: lsp.SymbolKind.Variable, docComment: node.comment?.docComment, symbolRange: toRange(node.span), nameRange: lsp.Range( start: lsp.Position( line: node.nameSpan.start.line, - // Include the $ in the range - character: node.nameSpan.start.column - 1), + // the span includes $ + character: node.nameSpan.start.column), end: lsp.Position( line: node.nameSpan.end.line, character: node.nameSpan.end.column))); diff --git a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart index 8d37138..ea61dfc 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart @@ -80,4 +80,114 @@ void main() { expect(symbolRange, EndsAtCharacter(12)); }); }); + + group('variables', () { + setUp(() { + ls.cache.clear(); + }); + + test('CSS variable ranges are correct', () { + var document = fs.createDocument(''' +.hello { + --world: blue; + color: var(--world); +} +'''); + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.children!.first.selectionRange; + var symbolRange = result.first.children!.first.range; + + expect(nameRange, StartsAtLine(1)); + expect(nameRange, StartsAtCharacter(2)); + + expect(nameRange, EndsAtLine(1)); + expect(nameRange, EndsAtCharacter(9)); + + expect(symbolRange, StartsAtLine(1)); + expect(symbolRange, StartsAtCharacter(2)); + + expect(symbolRange, EndsAtLine(1)); + expect(symbolRange, EndsAtCharacter(15)); // excluding ; + }); + + test('Sass variable ranges are correct', () { + var document = fs.createDocument(r''' +$world: blue; +.hello { + color: $world; +} +'''); + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.selectionRange; + var symbolRange = result.first.range; + + expect(nameRange, StartsAtLine(0)); + expect(nameRange, StartsAtCharacter(0)); + + expect(nameRange, EndsAtLine(0)); + expect(nameRange, EndsAtCharacter(6)); + + expect(symbolRange, StartsAtLine(0)); + expect(symbolRange, StartsAtCharacter(0)); + + expect(symbolRange, EndsAtLine(0)); + expect(symbolRange, EndsAtCharacter(12)); // excluding ; + }); + }); + + group('callable', () { + setUp(() { + ls.cache.clear(); + }); + + test('function ranges are correct', () { + var document = fs.createDocument(r''' +@function doStuff($a: 1, $b: 2) { + $value: $a + $b; + @return $value; +} +'''); + + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.selectionRange; + var symbolRange = result.first.range; + + expect(nameRange, StartsAtLine(0)); + expect(nameRange, StartsAtCharacter(10)); + + expect(nameRange, EndsAtLine(0)); + expect(nameRange, EndsAtCharacter(17)); + + expect(symbolRange, StartsAtLine(0)); + expect(symbolRange, StartsAtCharacter(0)); + + expect(symbolRange, EndsAtLine(3)); + expect(symbolRange, EndsAtCharacter(1)); + }); + + test('mixin ranges are correct', () { + var document = fs.createDocument(r''' +@mixin mixin1 { + $value: 1; + line-height: $value; +} +'''); + + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.selectionRange; + var symbolRange = result.first.range; + + expect(nameRange, StartsAtLine(0)); + expect(nameRange, StartsAtCharacter(7)); + + expect(nameRange, EndsAtLine(0)); + expect(nameRange, EndsAtCharacter(13)); + + expect(symbolRange, StartsAtLine(0)); + expect(symbolRange, StartsAtCharacter(0)); + + expect(symbolRange, EndsAtLine(3)); + expect(symbolRange, EndsAtCharacter(1)); + }); + }); } From 5a83b31ddb79aefd43ab19acc9e862607ca02199 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 12:02:06 +0100 Subject: [PATCH 16/23] Fix media and keyframes ranges --- .../document_symbols_visitor.dart | 20 +++- .../docyment_symbol_ranges_test.dart | 99 +++++++++++++++++++ 2 files changed, 115 insertions(+), 4 deletions(-) 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 3291ec5..2fdeaed 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 @@ -137,10 +137,11 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { var keyframesNameRange = lsp.Range( start: lsp.Position( line: node.name.span.start.line, - character: node.span.end.column + 1), + character: node.name.span.end.column + 1), end: lsp.Position( - line: node.span.end.line, - character: node.span.end.column + 1 + keyframesName.length)); + line: node.name.span.end.line, + character: + node.name.span.end.column + 1 + keyframesName.length)); _collect( name: keyframesName, @@ -186,11 +187,22 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { return; } + // node.query.span includes whitespace, so the range doesn't match node.query.asPlain + var nameRange = lsp.Range( + start: lsp.Position( + line: node.span.start.line + node.query.span.start.line, + character: node.span.start.column + node.query.span.start.column), + end: lsp.Position( + line: node.span.start.line + node.query.span.end.line, + character: node.span.start.column + + node.query.span.start.column + + node.query.asPlain!.length)); + _collect( name: '@media ${node.query.asPlain}', kind: lsp.SymbolKind.Module, symbolRange: toRange(node.span), - nameRange: toRange(node.query.span), + nameRange: nameRange, bodyRange: _bodyRange(node)); } diff --git a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart index ea61dfc..cb833b3 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart @@ -79,6 +79,29 @@ void main() { expect(symbolRange, EndsAtLine(1)); expect(symbolRange, EndsAtCharacter(12)); }); + + test('placeholder selector ranges are correct', () { + var document = fs.createDocument(''' +%waitforit { + color: red; +} +'''); + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.selectionRange; + var symbolRange = result.first.range; + + expect(nameRange, StartsAtLine(0)); + expect(nameRange, StartsAtCharacter(0)); + + expect(nameRange, EndsAtLine(0)); + expect(nameRange, EndsAtCharacter(10)); + + expect(symbolRange, StartsAtLine(0)); + expect(symbolRange, StartsAtCharacter(0)); + + expect(symbolRange, EndsAtLine(2)); + expect(symbolRange, EndsAtCharacter(1)); + }); }); group('variables', () { @@ -190,4 +213,80 @@ $world: blue; expect(symbolRange, EndsAtCharacter(1)); }); }); + + group('at-rules', () { + setUp(() { + ls.cache.clear(); + }); + + test('@media ranges are correct', () { + var document = fs.createDocument(r''' +@media screen, print + body + font-size: 14pt +''', uri: 'index.sass'); + + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.selectionRange; + var symbolRange = result.first.range; + + expect(nameRange, StartsAtLine(0)); + expect(nameRange, StartsAtCharacter(7)); + + expect(nameRange, EndsAtLine(0)); + expect(nameRange, EndsAtCharacter(20)); + + expect(symbolRange, StartsAtLine(0)); + expect(symbolRange, StartsAtCharacter(0)); + + expect(symbolRange, EndsAtLine(2)); + expect(symbolRange, EndsAtCharacter(19)); + }); + + test('@font-face ranges are correct', () { + var document = fs.createDocument(r''' +@font-face { + font-family: "Vulf Mono", monospace; +} +'''); + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.selectionRange; + var symbolRange = result.first.range; + + expect(nameRange, StartsAtLine(0)); + expect(nameRange, StartsAtCharacter(1)); + + expect(nameRange, EndsAtLine(0)); + expect(nameRange, EndsAtCharacter(10)); + + expect(symbolRange, StartsAtLine(0)); + expect(symbolRange, StartsAtCharacter(0)); + + expect(symbolRange, EndsAtLine(2)); + expect(symbolRange, EndsAtCharacter(1)); + }); + + test('@keyframes', () { + var document = fs.createDocument(r''' +@keyframes animation { + +} +'''); + var result = ls.findDocumentSymbols(document); + var nameRange = result.first.selectionRange; + var symbolRange = result.first.range; + + expect(nameRange, StartsAtLine(0)); + expect(nameRange, StartsAtCharacter(11)); + + expect(nameRange, EndsAtLine(0)); + expect(nameRange, EndsAtCharacter(20)); + + expect(symbolRange, StartsAtLine(0)); + expect(symbolRange, StartsAtCharacter(0)); + + expect(symbolRange, EndsAtLine(2)); + expect(symbolRange, EndsAtCharacter(1)); + }); + }); } From 4ed75a5bc47a9fe0462306c1934db628526ef747 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 13:16:57 +0100 Subject: [PATCH 17/23] Fix excludes for scanner and monorepo performance --- extension/package.json | 6 +-- .../lib/src/local_file_system.dart | 20 +++++++-- .../test/local_file_system_test.dart | 42 +++++++++++++++++++ .../workspace_configuration.dart | 9 +++- .../document_links_feature.dart | 8 +++- 5 files changed, 74 insertions(+), 11 deletions(-) create mode 100644 pkgs/sass_language_server/test/local_file_system_test.dart diff --git a/extension/package.json b/extension/package.json index 7f8e7c5..5b362a3 100644 --- a/extension/package.json +++ b/extension/package.json @@ -92,10 +92,10 @@ "type": "string" }, "default": [ - "**/.git/**", - "**/node_modules/**" + ".git/**", + "node_modules/**" ], - "description": "List of glob patterns for directories that are excluded when scanning.", + "description": "List of glob patterns for files that are excluded when scanning.", "order": 1 }, "sass.workspace.logLevel": { diff --git a/pkgs/sass_language_server/lib/src/local_file_system.dart b/pkgs/sass_language_server/lib/src/local_file_system.dart index f4c7198..6c92c3f 100644 --- a/pkgs/sass_language_server/lib/src/local_file_system.dart +++ b/pkgs/sass_language_server/lib/src/local_file_system.dart @@ -9,23 +9,35 @@ class LocalFileSystem extends FileSystemProvider { @override Future> findFiles(String pattern, {String? root, List? exclude}) async { - var list = - await Glob(pattern, caseSensitive: false).list(root: root).toList(); + var list = await Glob(pattern, caseSensitive: false) + .list( + root: root, + // Skip links. We resolve the real path behind links in findDocumentLinks later on. + // If we follow links here we can end up with an inflated initial list in + // monorepos that use symlinks in node_modules. + followLinks: false, + ) + .toList(); var excludeGlobs = []; if (exclude != null) { for (var pattern in exclude) { - excludeGlobs.add(Glob(pattern)); + excludeGlobs.add(Glob(pattern, caseSensitive: false)); } } var result = []; for (var match in list) { + var excluded = false; for (var glob in excludeGlobs) { if (glob.matches(match.path)) { - continue; + excluded = true; + break; } } + if (excluded) { + continue; + } result.add(match.uri); } diff --git a/pkgs/sass_language_server/test/local_file_system_test.dart b/pkgs/sass_language_server/test/local_file_system_test.dart new file mode 100644 index 0000000..f84bcd7 --- /dev/null +++ b/pkgs/sass_language_server/test/local_file_system_test.dart @@ -0,0 +1,42 @@ +import 'package:glob/glob.dart'; +import 'package:sass_language_services/sass_language_services.dart'; +import 'package:test/test.dart'; + +void main() { + group('Glob', () { + test('default exclude patterns match paths as expected', () { + var config = WorkspaceConfiguration.from(null); + var excludeGlobs = []; + for (var pattern in config.exclude) { + excludeGlobs.add(Glob(pattern, caseSensitive: false)); + } + + var nodeModulesPath = + '/home/user/workspace/project/node_modules/dependency/styles/index.scss'; + + var nodeModulesGlob = excludeGlobs.last; + + var matches = nodeModulesGlob.matches(nodeModulesPath); + expect(matches, isTrue); + }); + + test('user provided exclude patterns match paths as expected', () { + var config = WorkspaceConfiguration.from({ + 'exclude': ['node_modules/**'] + }); + + var excludeGlobs = []; + for (var pattern in config.exclude) { + excludeGlobs.add(Glob(pattern, caseSensitive: false)); + } + + var nodeModulesPath = + '/home/user/workspace/project/node_modules/dependency/styles/index.scss'; + + var nodeModulesGlob = excludeGlobs.first; + + var matches = nodeModulesGlob.matches(nodeModulesPath); + expect(matches, isTrue); + }); + }); +} diff --git a/pkgs/sass_language_services/lib/src/configuration/workspace_configuration.dart b/pkgs/sass_language_services/lib/src/configuration/workspace_configuration.dart index 636f30b..26f56cd 100644 --- a/pkgs/sass_language_services/lib/src/configuration/workspace_configuration.dart +++ b/pkgs/sass_language_services/lib/src/configuration/workspace_configuration.dart @@ -1,6 +1,6 @@ class WorkspaceConfiguration { /// Exclude paths from the initial workspace scan. Defaults include `.git` and `node_modules`. - final List exclude = ['**/.git/**', '**/node_modules/**']; + final List exclude = ['/**/.git/**', '/**/node_modules/**']; final Map importAliases = {}; /// Pass in [load paths](https://sass-lang.com/documentation/cli/dart-sass/#load-path) that will be used in addition to `node_modules`. @@ -15,7 +15,12 @@ class WorkspaceConfiguration { exclude.removeRange(0, exclude.length); for (var entry in excludeConfig) { if (entry is String) { - exclude.add(entry); + if (entry.startsWith('/')) { + exclude.add(entry); + } else { + // Paths we match against using Glob are absolute. + exclude.add('/**/$entry'); + } } } } diff --git a/pkgs/sass_language_services/lib/src/features/document_links/document_links_feature.dart b/pkgs/sass_language_services/lib/src/features/document_links/document_links_feature.dart index 460be98..1568233 100644 --- a/pkgs/sass_language_services/lib/src/features/document_links/document_links_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/document_links/document_links_feature.dart @@ -234,8 +234,12 @@ class DocumentLinksFeature extends LanguageFeature { final modulePath = await _resolvePathToModule(moduleName, documentFolder, rootFolder); if (modulePath != null) { - final pathWithinModule = target.path.substring(moduleName.length + 1); - return joinPath(modulePath, [pathWithinModule]); + if (target.path.length > moduleName.length + 1) { + final pathWithinModule = target.path.substring(moduleName.length + 1); + return joinPath(modulePath, [pathWithinModule]); + } else { + return modulePath; + } } return null; From 161ea0ba200cc3325d9d729052ac0a79a9874d51 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 14:01:48 +0100 Subject: [PATCH 18/23] Rework how links to Sass built-ins work Give them a target with a custom scheme so clients can choose how to handle them. --- extension/src/main.ts | 52 +++++++++++++++++++ .../lib/src/language_server.dart | 9 ++-- .../document_links_feature.dart | 4 +- 3 files changed, 60 insertions(+), 5 deletions(-) diff --git a/extension/src/main.ts b/extension/src/main.ts index 6e82233..af5a287 100644 --- a/extension/src/main.ts +++ b/extension/src/main.ts @@ -132,6 +132,58 @@ export async function activate(context: ExtensionContext): Promise { } } }); + + // TODO: Maybe worth looking into so links to built-ins resolve to something? + // workspace.registerFileSystemProvider( + // 'sass', + // { + // readFile(uri) { + // return Uint8Array.from( + // '@function hello();'.split('').map((c) => c.charCodeAt(0)) + // ); + // }, + // watch(uri, options) { + // return Disposable.create(() => { + // console.log('hello'); + // }); + // }, + // readDirectory(uri) { + // return []; + // }, + // stat(uri) { + // return { + // ctime: 0, + // mtime: 0, + // size: 0, + // type: 1, + // }; + // }, + // writeFile(uri, content, options) { + // return; + // }, + // createDirectory(uri) { + // return; + // }, + // delete(uri, options) { + // return; + // }, + // rename(oldUri, newUri, options) { + // return; + // }, + // copy(source, destination, options) { + // return; + // }, + // onDidChangeFile(e) { + // return Disposable.create(() => { + // console.log('hello'); + // }); + // }, + // }, + // { + // isCaseSensitive: false, + // isReadonly: true, + // } + // ); } export async function deactivate(): Promise { diff --git a/pkgs/sass_language_server/lib/src/language_server.dart b/pkgs/sass_language_server/lib/src/language_server.dart index 20904d9..689dc18 100644 --- a/pkgs/sass_language_server/lib/src/language_server.dart +++ b/pkgs/sass_language_server/lib/src/language_server.dart @@ -122,10 +122,13 @@ class LanguageServer { var links = await _ls.findDocumentLinks(document); for (var link in links) { if (link.target == null) continue; - if (link.target!.path.contains('#{')) continue; + + var target = link.target.toString(); + if (target.contains('#{')) continue; // Our findFiles glob will handle the initial parsing of CSS files - if (link.target!.path.endsWith('.css')) continue; - if (link.target!.path.startsWith('sass:')) continue; + if (target.endsWith('.css')) continue; + // Sass built-ins are not files we can scan. + if (target.startsWith('sass:')) continue; var visited = _ls.cache.getDocument(link.target as Uri); if (visited != null) { diff --git a/pkgs/sass_language_services/lib/src/features/document_links/document_links_feature.dart b/pkgs/sass_language_services/lib/src/features/document_links/document_links_feature.dart index 1568233..657736c 100644 --- a/pkgs/sass_language_services/lib/src/features/document_links/document_links_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/document_links/document_links_feature.dart @@ -41,10 +41,9 @@ class DocumentLinksFeature extends LanguageFeature { } if (target.startsWith('sass:')) { - // target is not included since this doesn't link to a file on disk - // TODO: https://github.com/sass/dart-sass-language-server/issues/5#issuecomment-2452932807 resolvedLinks.add(StylesheetDocumentLink( type: link.type, + target: Uri.parse('sass:///${link.namespace}.scss'), range: link.range, data: link.data, tooltip: link.tooltip, @@ -54,6 +53,7 @@ class DocumentLinksFeature extends LanguageFeature { shownVariables: link.shownVariables, hiddenMixinsAndFunctions: link.hiddenMixinsAndFunctions, shownMixinsAndFunctions: link.shownMixinsAndFunctions)); + resolvedLinks.add(link); continue; } From db46a8f37cd989d170ff6ea6e3a499f62d1b6a0e Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 14:03:17 +0100 Subject: [PATCH 19/23] Update test assertsions with new defaults --- pkgs/sass_language_services/test/configuration_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkgs/sass_language_services/test/configuration_test.dart b/pkgs/sass_language_services/test/configuration_test.dart index 5f6fc16..607df35 100644 --- a/pkgs/sass_language_services/test/configuration_test.dart +++ b/pkgs/sass_language_services/test/configuration_test.dart @@ -30,7 +30,7 @@ void main() { var result = LanguageServerConfiguration.create(null); expect(result.workspace.exclude, - equals(["**/.git/**", "**/node_modules/**"])); + equals(["/**/.git/**", "/**/node_modules/**"])); expect(result.workspace.loadPaths, isEmpty); expect(result.workspace.importAliases, isEmpty); @@ -50,7 +50,7 @@ void main() { // else defaults expect(result.workspace.exclude, - equals(["**/.git/**", "**/node_modules/**"])); + equals(["/**/.git/**", "/**/node_modules/**"])); expect(result.workspace.importAliases, isEmpty); }); }); From 386352d5659ab614ab1d2365d229e6cf64eb8515 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 15:16:54 +0100 Subject: [PATCH 20/23] Test path in cross-platform friendly way --- pkgs/sass_language_server/lib/src/language_server.dart | 3 +-- pkgs/sass_language_server/lib/src/local_file_system.dart | 6 ++++-- pkgs/sass_language_server/pubspec.yaml | 1 + pkgs/sass_language_server/test/local_file_system_test.dart | 7 +++++-- 4 files changed, 11 insertions(+), 6 deletions(-) diff --git a/pkgs/sass_language_server/lib/src/language_server.dart b/pkgs/sass_language_server/lib/src/language_server.dart index 689dc18..3a7dde5 100644 --- a/pkgs/sass_language_server/lib/src/language_server.dart +++ b/pkgs/sass_language_server/lib/src/language_server.dart @@ -211,8 +211,7 @@ class LanguageServer { } _log.debug('Searching workspace for files'); - var files = await fileSystemProvider.findFiles( - '**/*.{css,scss,sass}', + var files = await fileSystemProvider.findFiles('**.{css,scss,sass}', root: _workspaceRoot.toFilePath(), exclude: _ls.configuration.workspace.exclude); _log.debug('Found ${files.length} files in workspace'); diff --git a/pkgs/sass_language_server/lib/src/local_file_system.dart b/pkgs/sass_language_server/lib/src/local_file_system.dart index 6c92c3f..d50601c 100644 --- a/pkgs/sass_language_server/lib/src/local_file_system.dart +++ b/pkgs/sass_language_server/lib/src/local_file_system.dart @@ -2,6 +2,7 @@ import 'dart:io'; import 'package:glob/glob.dart'; import 'package:glob/list_local_fs.dart'; +import 'package:path/path.dart' as p; import 'package:sass_language_server/src/utils/uri.dart'; import 'package:sass_language_services/sass_language_services.dart'; @@ -22,7 +23,8 @@ class LocalFileSystem extends FileSystemProvider { var excludeGlobs = []; if (exclude != null) { for (var pattern in exclude) { - excludeGlobs.add(Glob(pattern, caseSensitive: false)); + excludeGlobs.add(Glob(pattern, + caseSensitive: false, context: p.Context(style: p.Style.url))); } } @@ -30,7 +32,7 @@ class LocalFileSystem extends FileSystemProvider { for (var match in list) { var excluded = false; for (var glob in excludeGlobs) { - if (glob.matches(match.path)) { + if (glob.matches(match.uri.path)) { excluded = true; break; } diff --git a/pkgs/sass_language_server/pubspec.yaml b/pkgs/sass_language_server/pubspec.yaml index 97e89e9..23572fc 100644 --- a/pkgs/sass_language_server/pubspec.yaml +++ b/pkgs/sass_language_server/pubspec.yaml @@ -13,6 +13,7 @@ environment: dependencies: lsp_server: ^0.3.2 glob: ^2.1.2 + path: ^1.9.0 sass_language_services: ^1.0.0 dev_dependencies: diff --git a/pkgs/sass_language_server/test/local_file_system_test.dart b/pkgs/sass_language_server/test/local_file_system_test.dart index f84bcd7..4400a3f 100644 --- a/pkgs/sass_language_server/test/local_file_system_test.dart +++ b/pkgs/sass_language_server/test/local_file_system_test.dart @@ -1,4 +1,5 @@ import 'package:glob/glob.dart'; +import 'package:path/path.dart' as p; import 'package:sass_language_services/sass_language_services.dart'; import 'package:test/test.dart'; @@ -8,7 +9,8 @@ void main() { var config = WorkspaceConfiguration.from(null); var excludeGlobs = []; for (var pattern in config.exclude) { - excludeGlobs.add(Glob(pattern, caseSensitive: false)); + excludeGlobs.add(Glob(pattern, + caseSensitive: false, context: p.Context(style: p.Style.url))); } var nodeModulesPath = @@ -27,7 +29,8 @@ void main() { var excludeGlobs = []; for (var pattern in config.exclude) { - excludeGlobs.add(Glob(pattern, caseSensitive: false)); + excludeGlobs.add(Glob(pattern, + caseSensitive: false, context: p.Context(style: p.Style.url))); } var nodeModulesPath = From ae787868957eb205d3c79f1b296b4bb86e72e2ab Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 15:49:23 +0100 Subject: [PATCH 21/23] Review nits [skip ci] --- extension/test/README.md | 2 +- extension/test/electron/document-links/fixtures/styles.scss | 6 ------ 2 files changed, 1 insertion(+), 7 deletions(-) diff --git a/extension/test/README.md b/extension/test/README.md index 1c743ed..78dfd95 100644 --- a/extension/test/README.md +++ b/extension/test/README.md @@ -6,7 +6,7 @@ See [the list of built-in commands](https://code.visualstudio.com/api/references The runner is configured so it can run multiple instances of VS Code (in sequence) using different configurations and workspaces. Each subdirectory in `electron/` will run in a separate instance of VS Code. -By convention subdirectories in `electron/` must have: +By convention subdirectories in `electron/` should have: - `index.js` as the entrypoint that finds test files and passes them to Mocha - at least one `*.test.js` file with some tests diff --git a/extension/test/electron/document-links/fixtures/styles.scss b/extension/test/electron/document-links/fixtures/styles.scss index 1d54d4c..0d32c36 100644 --- a/extension/test/electron/document-links/fixtures/styles.scss +++ b/extension/test/electron/document-links/fixtures/styles.scss @@ -4,9 +4,3 @@ button { color: colors.$color-primary; } - -@media screen, print { - body { - font-size: 14pt; - } -} From 07d63ec097930039ba871b449da267f4ea56c971 Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 16:26:47 +0100 Subject: [PATCH 22/23] Clean up unused code, cache --- .../document_symbols_feature.dart | 7 +++ .../document_symbols_visitor.dart | 44 ++++--------------- .../lib/src/language_services_cache.dart | 10 +++++ 3 files changed, 25 insertions(+), 36 deletions(-) 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 0c2966d..b2ab6a9 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,11 +7,18 @@ 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/document_symbols/document_symbols_visitor.dart b/pkgs/sass_language_services/lib/src/features/document_symbols/document_symbols_visitor.dart index 2fdeaed..706f24b 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 @@ -23,7 +23,6 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { required lsp.SymbolKind kind, required lsp.Range symbolRange, lsp.Range? nameRange, - lsp.Range? bodyRange, // TODO: delete if unused String? docComment, String? detail, List? tags}) { @@ -49,9 +48,6 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { var keep = []; for (var other in symbols) { - // This probably scales terribly with document size. - // A tree structure that uses ranges for lookup would probably be the ticket here. - // We can maybe get away with it if we cache the result. if (_containsRange(symbol.range, other.range)) { symbol.children!.add(other); } else { @@ -90,24 +86,6 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { return false; } - lsp.Range? _bodyRange(sass.ParentStatement?> node) { - if (node.children case var children?) { - if (children.isEmpty) { - return null; - } - - return lsp.Range( - start: lsp.Position( - line: children.first.span.start.line, - character: children.first.span.start.column), - end: lsp.Position( - line: children.last.span.start.line, - character: children.last.span.start.column)); - } - - return null; - } - String? _detail(sass.CallableDeclaration node) { var arguments = node.arguments.arguments .map((arg) => arg.defaultValue != null @@ -120,6 +98,7 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { @override void visitAtRule(node) { super.visitAtRule(node); + if (!node.name.isPlain) { return; } @@ -129,8 +108,7 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { name: node.name.span.text, kind: lsp.SymbolKind.Class, symbolRange: toRange(node.span), - nameRange: toRange(node.name.span), - bodyRange: _bodyRange(node)); + nameRange: toRange(node.name.span)); } else if (node.name.asPlain!.startsWith('keyframes')) { var keyframesName = node.span.context.split(' ').elementAtOrNull(1); if (keyframesName != null) { @@ -147,8 +125,7 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { name: keyframesName, kind: lsp.SymbolKind.Class, symbolRange: toRange(node.span), - nameRange: keyframesNameRange, - bodyRange: _bodyRange(node)); + nameRange: keyframesNameRange); } } } @@ -161,8 +138,7 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { name: node.name.span.text, kind: lsp.SymbolKind.Variable, symbolRange: toRange(node.span), - nameRange: toRange(node.name.span), - bodyRange: _bodyRange(node)); + nameRange: toRange(node.name.span)); } } @@ -176,8 +152,7 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { kind: lsp.SymbolKind.Function, docComment: node.comment?.docComment, symbolRange: toRange(node.span), - nameRange: toRange(node.nameSpan), - bodyRange: _bodyRange(node)); + nameRange: toRange(node.nameSpan)); } @override @@ -202,8 +177,7 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { name: '@media ${node.query.asPlain}', kind: lsp.SymbolKind.Module, symbolRange: toRange(node.span), - nameRange: nameRange, - bodyRange: _bodyRange(node)); + nameRange: nameRange); } @override @@ -216,8 +190,7 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { kind: lsp.SymbolKind.Method, docComment: node.comment?.docComment, symbolRange: toRange(node.span), - nameRange: toRange(node.nameSpan), - bodyRange: _bodyRange(node)); + nameRange: toRange(node.nameSpan)); } @override @@ -279,8 +252,7 @@ class DocumentSymbolsVisitor with sass.RecursiveStatementVisitor { name: name!, kind: lsp.SymbolKind.Class, symbolRange: symbolRange!, - nameRange: nameRange, - bodyRange: _bodyRange(node)); + nameRange: nameRange); } } on sass.SassFormatException catch (_) { // Do nothing. 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 91b256f..2b2a3c2 100644 --- a/pkgs/sass_language_services/lib/src/language_services_cache.dart +++ b/pkgs/sass_language_services/lib/src/language_services_cache.dart @@ -5,6 +5,7 @@ class CacheEntry { TextDocument document; sass.Stylesheet stylesheet; List? links; + List? symbols; CacheEntry({ required this.document, @@ -52,11 +53,20 @@ class LanguageServicesCache { return _cache[document.uri.toString()]?.links; } + List? getDocumentSymbols(TextDocument document) { + return _cache[document.uri.toString()]?.symbols; + } + void setDocumentLinks( TextDocument document, List links) { _cache[document.uri.toString()]?.links = links; } + void setDocumentSymbols( + TextDocument document, List symbols) { + _cache[document.uri.toString()]?.symbols = symbols; + } + Iterable getDocuments() { return _cache.values.map((e) => e.document); } From 0b98015e9d5ce6eae3c59a07a99266476e455d1b Mon Sep 17 00:00:00 2001 From: William Killerud Date: Sun, 10 Nov 2024 16:28:18 +0100 Subject: [PATCH 23/23] Refactor to match test style --- .../docyment_symbol_ranges_test.dart | 34 +++++++++---------- ...tion_matchers.dart => range_matchers.dart} | 14 -------- 2 files changed, 17 insertions(+), 31 deletions(-) rename pkgs/sass_language_services/test/{position_matchers.dart => range_matchers.dart} (70%) diff --git a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart index cb833b3..95cb92c 100644 --- a/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart +++ b/pkgs/sass_language_services/test/features/document_symbols/docyment_symbol_ranges_test.dart @@ -2,7 +2,7 @@ import 'package:sass_language_services/sass_language_services.dart'; import 'package:test/test.dart'; import '../../memory_file_system.dart'; -import '../../position_matchers.dart'; +import '../../range_matchers.dart'; import '../../test_client_capabilities.dart'; final fs = MemoryFileSystem(); @@ -28,32 +28,32 @@ void main() { var result = ls.findDocumentSymbols(document); var fooNameRange = result.first.selectionRange; var fooSymbolRange = result.first.range; - expect(fooNameRange.start, AtLine(0)); - expect(fooNameRange.start, AtCharacter(0)); + expect(fooNameRange, StartsAtLine(0)); + expect(fooNameRange, StartsAtCharacter(0)); - expect(fooNameRange.end, AtLine(0)); - expect(fooNameRange.end, AtCharacter(4)); + expect(fooNameRange, EndsAtLine(0)); + expect(fooNameRange, EndsAtCharacter(4)); - expect(fooSymbolRange.start, AtLine(0)); - expect(fooSymbolRange.start, AtCharacter(0)); + expect(fooSymbolRange, StartsAtLine(0)); + expect(fooSymbolRange, StartsAtCharacter(0)); - expect(fooSymbolRange.end, AtLine(2)); - expect(fooSymbolRange.end, AtCharacter(1)); + expect(fooSymbolRange, EndsAtLine(2)); + expect(fooSymbolRange, EndsAtCharacter(1)); var barNameRange = result.last.selectionRange; var barSymbolRange = result.last.range; - expect(barNameRange.start, AtLine(4)); - expect(barNameRange.start, AtCharacter(0)); + expect(barNameRange, StartsAtLine(4)); + expect(barNameRange, StartsAtCharacter(0)); - expect(barNameRange.end, AtLine(4)); - expect(barNameRange.end, AtCharacter(4)); + expect(barNameRange, EndsAtLine(4)); + expect(barNameRange, EndsAtCharacter(4)); - expect(barSymbolRange.start, AtLine(4)); - expect(barSymbolRange.start, AtCharacter(0)); + expect(barSymbolRange, StartsAtLine(4)); + expect(barSymbolRange, StartsAtCharacter(0)); - expect(barSymbolRange.end, AtLine(6)); - expect(barSymbolRange.end, AtCharacter(1)); + expect(barSymbolRange, EndsAtLine(6)); + expect(barSymbolRange, EndsAtCharacter(1)); }); test('Sass indented selector ranges are correct', () { diff --git a/pkgs/sass_language_services/test/position_matchers.dart b/pkgs/sass_language_services/test/range_matchers.dart similarity index 70% rename from pkgs/sass_language_services/test/position_matchers.dart rename to pkgs/sass_language_services/test/range_matchers.dart index 967df2b..41bd43d 100644 --- a/pkgs/sass_language_services/test/position_matchers.dart +++ b/pkgs/sass_language_services/test/range_matchers.dart @@ -1,20 +1,6 @@ import 'package:lsp_server/lsp_server.dart'; import 'package:test/test.dart'; -class AtLine extends CustomMatcher { - AtLine(Object? valueOrMatcher) - : super('Position at line', 'line', valueOrMatcher); - - featureValueOf(actual) => (actual as Position).line; -} - -class AtCharacter extends CustomMatcher { - AtCharacter(Object? valueOrMatcher) - : super('Position at character', 'character', valueOrMatcher); - - featureValueOf(actual) => (actual as Position).character; -} - class StartsAtLine extends CustomMatcher { StartsAtLine(Object? valueOrMatcher) : super('Range starts at line', 'line', valueOrMatcher);