diff --git a/pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart b/pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart index 38955e5..8bba3c2 100644 --- a/pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart @@ -93,9 +93,7 @@ class FindReferencesFeature extends GoToDefinitionFeature { definition.location!, ); - if (!context.includeDeclaration && candidateIsDefinition) { - continue; - } else if (candidateIsDefinition) { + if (candidateIsDefinition) { references.add(candidate); continue; } diff --git a/pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart b/pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart index d269ec1..14d6e7a 100644 --- a/pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart +++ b/pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart @@ -41,28 +41,9 @@ class FindReferencesVisitor super.visitExtendRule(node); } - lsp.Range _getForwardVisibilityRange(sass.ForwardRule node, String name) { - var nameIndex = node.span.text.indexOf( - name, - node.span.start.offset - node.urlSpan.end.offset, - ); - - var selectionRange = lsp.Range( - start: lsp.Position( - line: node.span.start.line, - character: node.span.start.column + nameIndex, - ), - end: lsp.Position( - line: node.span.start.line, - character: node.span.start.column + nameIndex + name.length, - ), - ); - return selectionRange; - } - @override void visitForwardRule(sass.ForwardRule node) { - // TODO: would be nice to have span information for forward visibility from sass_api. + // TODO: would be nice to have span information for forward visibility from sass_api. Even nicer if we could tell at this point wheter something is a mixin or a function. if (node.hiddenMixinsAndFunctions case var hiddenMixinsAndFunctions?) { for (var name in hiddenMixinsAndFunctions) { @@ -70,7 +51,7 @@ class FindReferencesVisitor continue; } - var selectionRange = _getForwardVisibilityRange(node, name); + var selectionRange = forwardVisibilityRange(node, name); var location = lsp.Location(range: selectionRange, uri: _document.uri); // We can't tell if this is a mixin or a function, so add a candidate for both. @@ -97,7 +78,7 @@ class FindReferencesVisitor continue; } - var selectionRange = _getForwardVisibilityRange(node, name); + var selectionRange = forwardVisibilityRange(node, '\$$name'); var location = lsp.Location(range: selectionRange, uri: _document.uri); candidates.add( @@ -116,7 +97,7 @@ class FindReferencesVisitor continue; } - var selectionRange = _getForwardVisibilityRange(node, name); + var selectionRange = forwardVisibilityRange(node, name); var location = lsp.Location(range: selectionRange, uri: _document.uri); // We can't tell if this is a mixin or a function, so add a candidate for both. @@ -143,7 +124,7 @@ class FindReferencesVisitor continue; } - var selectionRange = _getForwardVisibilityRange(node, name); + var selectionRange = forwardVisibilityRange(node, '\$$name'); var location = lsp.Location(range: selectionRange, uri: _document.uri); candidates.add( @@ -163,6 +144,7 @@ class FindReferencesVisitor void visitFunctionExpression(sass.FunctionExpression node) { var name = node.name; if (!name.contains(_name)) { + super.visitFunctionExpression(node); return; } var location = lsp.Location( @@ -182,10 +164,12 @@ class FindReferencesVisitor @override void visitFunctionRule(sass.FunctionRule node) { if (!_includeDeclaration) { + super.visitFunctionRule(node); return; } var name = node.name; if (!name.contains(_name)) { + super.visitFunctionRule(node); return; } var location = lsp.Location( @@ -206,6 +190,7 @@ class FindReferencesVisitor void visitIncludeRule(sass.IncludeRule node) { var name = node.name; if (!name.contains(_name)) { + super.visitIncludeRule(node); return; } var location = lsp.Location( @@ -225,10 +210,12 @@ class FindReferencesVisitor @override void visitMixinRule(sass.MixinRule node) { if (!_includeDeclaration) { + super.visitMixinRule(node); return; } var name = node.name; if (!name.contains(_name)) { + super.visitMixinRule(node); return; } var location = lsp.Location( @@ -248,6 +235,7 @@ class FindReferencesVisitor @override void visitStyleRule(sass.StyleRule node) { if (!_includeDeclaration) { + super.visitStyleRule(node); return; } @@ -289,10 +277,12 @@ class FindReferencesVisitor @override void visitVariableDeclaration(sass.VariableDeclaration node) { if (!_includeDeclaration) { + super.visitVariableDeclaration(node); return; } var name = node.name; if (!name.contains(_name)) { + super.visitVariableDeclaration(node); return; } var location = lsp.Location( @@ -313,6 +303,7 @@ class FindReferencesVisitor void visitVariableExpression(sass.VariableExpression node) { var name = node.name; if (!name.contains(_name)) { + super.visitVariableExpression(node); return; } var location = lsp.Location( diff --git a/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart b/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart index 383bd20..d940f2f 100644 --- a/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart +++ b/pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart @@ -1,9 +1,11 @@ import 'package:lsp_server/lsp_server.dart' as lsp; import 'package:sass_api/sass_api.dart' as sass; import 'package:sass_language_services/sass_language_services.dart'; +import 'package:sass_language_services/src/features/find_references/reference.dart'; import 'package:sass_language_services/src/features/go_to_definition/scoped_symbols.dart'; import 'package:sass_language_services/src/features/node_at_offset_visitor.dart'; +import '../../utils/sass_lsp_utils.dart'; import '../language_feature.dart'; import 'definition.dart'; import 'scope_visitor.dart'; @@ -34,30 +36,51 @@ class GoToDefinitionFeature extends LanguageFeature { return null; } - // Get the node's ReferenceKind and name so we can compare it to other symbols. - var kind = getNodeReferenceKind(node); - if (kind == null) { + // The visibility configuration needs special handling. + // We don't always know if something refers to a function or mixin, so + // we check for both kinds. Only relevant for the workspace traversal though. + String? name; + var kinds = []; + if (node is sass.ForwardRule) { + var result = _getForwardVisibilityCandidates(node, position); + if (result != null) { + (name, kinds) = result; + } + } else { + // Get the node's ReferenceKind and name so we can compare it to other symbols. + var kind = getNodeReferenceKind(node); + if (kind == null) { + return null; + } + kinds = [kind]; + + name = getNodeName(node); + if (name == null) { + return null; + } + + // Look for the symbol in the current document. + // It may be a scoped symbol. + var symbols = ScopedSymbols(stylesheet, + document.languageId == 'sass' ? Dialect.indented : Dialect.scss); + var symbol = symbols.findSymbolFromNode(node); + if (symbol != null) { + // Found the definition in the same document. + return Definition( + name, + kind, + lsp.Location(uri: document.uri, range: symbol.selectionRange), + ); + } + } + + if (kinds.isEmpty) { return null; } - var name = getNodeName(node); if (name == null) { return null; } - // Look for the symbol in the current document. - // It may be a scoped symbol. - var symbols = ScopedSymbols(stylesheet, - document.languageId == 'sass' ? Dialect.indented : Dialect.scss); - var symbol = symbols.findSymbolFromNode(node); - if (symbol != null) { - // Found the definition in the same document. - return Definition( - name, - kind, - lsp.Location(uri: document.uri, range: symbol.selectionRange), - ); - } - // Start looking from the linked document In case of a namespace // so we don't accidentally match with a symbol of the same kind // and name, but in a different module. @@ -96,31 +119,32 @@ class GoToDefinitionFeature extends LanguageFeature { required List shownMixinsAndFunctions, required List shownVariables, }) async { - // `@forward` may add a prefix to [name], - // but we're comparing it to symbols without that prefix. - var unprefixedName = kind == ReferenceKind.function || - kind == ReferenceKind.mixin || - kind == ReferenceKind.variable - ? name.replaceFirst(prefix, '') - : name; - - var stylesheet = ls.parseStylesheet(document); - var symbols = ScopedSymbols(stylesheet, - document.languageId == 'sass' ? Dialect.indented : Dialect.scss); - var symbol = symbols.globalScope.getSymbol( - name: unprefixedName, - referenceKind: kind, - ); + for (var kind in kinds) { + // `@forward` may add a prefix to [name], + // but we're comparing it to symbols without that prefix. + var unprefixedName = kind == ReferenceKind.function || + kind == ReferenceKind.mixin || + kind == ReferenceKind.variable + ? name!.replaceFirst(prefix, '') + : name!; + + var stylesheet = ls.parseStylesheet(document); + var symbols = ScopedSymbols(stylesheet, + document.languageId == 'sass' ? Dialect.indented : Dialect.scss); + var symbol = symbols.globalScope.getSymbol( + name: unprefixedName, + referenceKind: kind, + ); - if (symbol != null) { - return [ - ( - symbol, - lsp.Location(uri: document.uri, range: symbol.selectionRange) - ) - ]; + if (symbol != null) { + return [ + ( + symbol, + lsp.Location(uri: document.uri, range: symbol.selectionRange) + ) + ]; + } } - return null; }, ); @@ -139,17 +163,59 @@ class GoToDefinitionFeature extends LanguageFeature { for (var document in ls.cache.getDocuments()) { var symbols = ls.findDocumentSymbols(document); for (var symbol in symbols) { - if (symbol.name == name && symbol.referenceKind == kind) { - return Definition( - name, - kind, - lsp.Location(uri: document.uri, range: symbol.selectionRange), - ); + for (var kind in kinds) { + if (symbol.name == name && symbol.referenceKind == kind) { + return Definition( + name, + kind, + lsp.Location(uri: document.uri, range: symbol.selectionRange), + ); + } } } } // Could be a Sass built-in module. - return Definition(name, kind, null); + return Definition(name, kinds.first, null); + } + + (String, List)? _getForwardVisibilityCandidates( + sass.ForwardRule node, lsp.Position position) { + if (node.hiddenMixinsAndFunctions case var hiddenMixinsAndFunctions?) { + for (var name in hiddenMixinsAndFunctions) { + var selectionRange = forwardVisibilityRange(node, name); + if (isInRange(position: position, range: selectionRange)) { + return (name, [ReferenceKind.function, ReferenceKind.mixin]); + } + } + } + + if (node.hiddenVariables case var hiddenVariables?) { + for (var name in hiddenVariables) { + var selectionRange = forwardVisibilityRange(node, '\$$name'); + if (isInRange(position: position, range: selectionRange)) { + return (name, [ReferenceKind.variable]); + } + } + } + + if (node.shownMixinsAndFunctions case var shownMixinsAndFunctions?) { + for (var name in shownMixinsAndFunctions) { + var selectionRange = forwardVisibilityRange(node, name); + if (isInRange(position: position, range: selectionRange)) { + return (name, [ReferenceKind.function, ReferenceKind.mixin]); + } + } + } + + if (node.shownVariables case var shownVariables?) { + for (var name in shownVariables) { + var selectionRange = forwardVisibilityRange(node, '\$$name'); + if (isInRange(position: position, range: selectionRange)) { + return (name, [ReferenceKind.variable]); + } + } + } + return null; } } diff --git a/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart b/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart index 571081d..43975df 100644 --- a/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart +++ b/pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart @@ -25,3 +25,29 @@ lsp.Range selectorNameRange( ), ); } + +lsp.Range forwardVisibilityRange(sass.ForwardRule node, String name) { + var nameIndex = node.span.text.indexOf( + name, + node.span.start.offset + node.urlSpan.end.offset, + ); + + var selectionRange = lsp.Range( + start: lsp.Position( + line: node.span.start.line, + character: node.span.start.column + nameIndex, + ), + end: lsp.Position( + line: node.span.start.line, + character: node.span.start.column + nameIndex + name.length, + ), + ); + return selectionRange; +} + +bool isInRange({required lsp.Position position, required lsp.Range range}) { + return range.start.line <= position.line && + range.start.character <= position.character && + range.end.line >= position.line && + range.end.character >= position.character; +} diff --git a/pkgs/sass_language_services/test/features/find_references/find_references_test.dart b/pkgs/sass_language_services/test/features/find_references/find_references_test.dart new file mode 100644 index 0000000..406ab56 --- /dev/null +++ b/pkgs/sass_language_services/test/features/find_references/find_references_test.dart @@ -0,0 +1,178 @@ +import 'package:lsp_server/lsp_server.dart' as lsp; +import 'package:sass_language_services/sass_language_services.dart'; +import 'package:test/test.dart'; + +import '../../memory_file_system.dart'; +import '../../position_utils.dart'; +import '../../range_matchers.dart'; +import '../../test_client_capabilities.dart'; + +final fs = MemoryFileSystem(); +final ls = LanguageServices(fs: fs, clientCapabilities: getCapabilities()); +final context = lsp.ReferenceContext(includeDeclaration: true); + +void main() { + group('sass variables', () { + setUp(() { + ls.cache.clear(); + }); + + test('finds variable references in scope', () async { + var document = fs.createDocument(r''' +.a + $b: blue + color: $b +''', uri: 'styles.sass'); + var result = + await ls.findReferences(document, at(line: 2, char: 10), context); + + expect(result, hasLength(2)); + + var [first, second] = result; + expect(first.range, StartsAtLine(1)); + expect(first.range, EndsAtLine(1)); + expect(first.range, StartsAtCharacter(2)); + expect(first.range, EndsAtCharacter(4)); + + expect(second.range, StartsAtLine(2)); + expect(second.range, EndsAtLine(2)); + expect(second.range, StartsAtCharacter(9)); + expect(second.range, EndsAtCharacter(11)); + }); + + test('exclude declaration at user request', () async { + var document = fs.createDocument(r''' +.a + $b: blue + color: $b +''', uri: 'styles.sass'); + var result = await ls.findReferences( + document, + at(line: 2, char: 10), + lsp.ReferenceContext(includeDeclaration: false), + ); + + expect(result, hasLength(1)); + + var [first] = result; + expect(first.range, StartsAtLine(2)); + expect(first.range, EndsAtLine(2)); + expect(first.range, StartsAtCharacter(9)); + expect(first.range, EndsAtCharacter(11)); + }); + + test('finds variable references across workspace', () async { + var ki = fs.createDocument(r''' +$day: "monday"; +''', uri: 'ki.scss'); + + var helen = fs.createDocument(r''' +@use "ki"; + +.a::after { + content: ki.$day; +} +''', uri: 'helen.scss'); + + var document = fs.createDocument(r''' +@use "ki" + +.a::before + // Here it comes! + content: ki.$day +''', uri: 'gato.sass'); + + // Emulate the language server's initial scan. + // Needed since gato does not have helen in its + // module tree, but they both reference the same + // variable. + ls.parseStylesheet(ki); + ls.parseStylesheet(helen); + + var result = + await ls.findReferences(document, at(line: 4, char: 16), context); + + expect(result, hasLength(3)); + + var [first, second, third] = result; + expect(first.uri.toString(), endsWith('ki.scss')); + expect(first.range, StartsAtLine(0)); + expect(first.range, EndsAtLine(0)); + expect(first.range, StartsAtCharacter(0)); + expect(first.range, EndsAtCharacter(4)); + + expect(second.uri.toString(), endsWith('helen.scss')); + expect(second.range, StartsAtLine(3)); + expect(second.range, EndsAtLine(3)); + expect(second.range, StartsAtCharacter(14)); + expect(second.range, EndsAtCharacter(18)); + + expect(third.uri.toString(), endsWith('gato.sass')); + expect(third.range, StartsAtLine(4)); + expect(third.range, EndsAtLine(4)); + expect(third.range, StartsAtCharacter(14)); + expect(third.range, EndsAtCharacter(18)); + }); + + test('finds variable with prefix and in visibility modifier', () async { + var ki = fs.createDocument(r''' +$day: "monday"; +''', uri: 'ki.scss'); + var dev = fs.createDocument(r''' +@forward "ki" as ki-* show $day; +''', uri: 'dev.scss'); + + var helen = fs.createDocument(r''' +@use "dev"; + +.a::after { + content: dev.$ki-day; +} +''', uri: 'helen.scss'); + var gato = fs.createDocument(r''' +@use "ki"; + +.a::before { + content: ki.$day; +} +''', uri: 'gato.scss'); + + // Emulate the language server's initial scan. + // Needed since the stylesheets don't all have eachother in their + // module tree, but they all reference the same variable. + ls.parseStylesheet(ki); + ls.parseStylesheet(dev); + ls.parseStylesheet(helen); + + var result = + await ls.findReferences(gato, at(line: 3, char: 15), context); + + expect(result, hasLength(4)); + + var [first, second, third, fourth] = result; + expect(first.uri.toString(), endsWith('ki.scss')); + expect(first.range, StartsAtLine(0)); + expect(first.range, EndsAtLine(0)); + expect(first.range, StartsAtCharacter(0)); + expect(first.range, EndsAtCharacter(4)); + + expect(second.uri.toString(), endsWith('dev.scss')); + expect(second.range, StartsAtLine(0)); + expect(second.range, EndsAtLine(0)); + expect(second.range, StartsAtCharacter(27)); + expect(second.range, EndsAtCharacter(31)); + + expect(third.uri.toString(), endsWith('helen.scss')); + expect(third.range, StartsAtLine(3)); + expect(third.range, EndsAtLine(3)); + expect(third.range, StartsAtCharacter(15)); + expect(third.range, EndsAtCharacter(22)); + + expect(fourth.uri.toString(), endsWith('gato.scss')); + expect(fourth.range, StartsAtLine(3)); + expect(fourth.range, EndsAtLine(3)); + expect(fourth.range, StartsAtCharacter(14)); + expect(fourth.range, EndsAtCharacter(18)); + }); + }); +} diff --git a/pkgs/sass_language_services/test/features/go_to_definition/go_to_definition_test.dart b/pkgs/sass_language_services/test/features/go_to_definition/go_to_definition_test.dart index a570109..e643e9f 100644 --- a/pkgs/sass_language_services/test/features/go_to_definition/go_to_definition_test.dart +++ b/pkgs/sass_language_services/test/features/go_to_definition/go_to_definition_test.dart @@ -103,6 +103,24 @@ $b: blue expect(result, isNull); }); + + test('forward visibility', () async { + fs.createDocument(r''' +$day: "monday"; +''', uri: 'ki.scss'); + var dev = fs.createDocument(r''' +@forward "ki" as ki-* show $day; +''', uri: 'dev.scss'); + + var result = await ls.goToDefinition(dev, at(line: 0, char: 28)); + + expect(result, isNotNull); + expect(result!.range, StartsAtLine(0)); + expect(result.range, EndsAtLine(0)); + expect(result.range, StartsAtCharacter(0)); + expect(result.range, EndsAtCharacter(4)); + expect(result.uri.toString(), endsWith('ki.scss')); + }); }); group('mixins', () { @@ -214,6 +232,37 @@ nav ul { expect(result.uri.toString(), endsWith('_list.sass')); }); + + test('forward visibility', () async { + fs.createDocument(r''' +=reset-list + margin: 0 + padding: 0 + list-style: none + +=horizontal-list + +reset-list + + li + display: inline-block + margin: + left: -2px + right: 2em +''', uri: '_list.sass'); + + var document = fs.createDocument(r''' +@forward "list" as list-* hide reset-list +''', uri: 'shared.sass'); + + var result = await ls.goToDefinition(document, at(line: 0, char: 32)); + + expect(result, isNotNull); + expect(result!.range, StartsAtLine(0)); + expect(result.range, EndsAtLine(0)); + expect(result.range, StartsAtCharacter(1)); + expect(result.range, EndsAtCharacter(11)); + expect(result.uri.toString(), endsWith('_list.sass')); + }); }); group('sass functions', () {