Skip to content

Commit

Permalink
Fix issues with forward visibility
Browse files Browse the repository at this point in the history
  • Loading branch information
@wkillerud
wkillerud committed Nov 23, 2024
1 parent 76159a9 commit ad537c3
Show file tree
Hide file tree
Showing 6 changed files with 383 additions and 75 deletions.
4 changes: 1 addition & 3 deletions pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,7 @@ class FindReferencesFeature extends GoToDefinitionFeature {
definition.location!,
);

if (!context.includeDeclaration && candidateIsDefinition) {
continue;
} else if (candidateIsDefinition) {
if (candidateIsDefinition) {
references.add(candidate);
continue;
}
Expand Down
39 changes: 15 additions & 24 deletions pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,36 +41,17 @@ 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) {
if (!name.contains(_name)) {
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.
Expand All @@ -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(
Expand All @@ -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.
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand All @@ -248,6 +235,7 @@ class FindReferencesVisitor
@override
void visitStyleRule(sass.StyleRule node) {
if (!_includeDeclaration) {
super.visitStyleRule(node);
return;
}

Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand Down
162 changes: 114 additions & 48 deletions pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart
View file
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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 = <ReferenceKind>[];
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.
Expand Down Expand Up @@ -96,31 +119,32 @@ class GoToDefinitionFeature extends LanguageFeature {
required List<String> shownMixinsAndFunctions,
required List<String> 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;
},
);
Expand All @@ -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<ReferenceKind>)? _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;
}
}
26 changes: 26 additions & 0 deletions pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Loading

0 comments on commit ad537c3

Please sign in to comment.