Skip to content

Commit a82268e

Browse files
wkillerudwkillerud
committed
Fix issues with forward visibility
1 parent eec1381 commit a82268e

File tree

6 files changed

+383
-75
lines changed

6 files changed

+383
-75
lines changed

‎pkgs/sass_language_services/lib/src/features/find_references/find_references_feature.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,7 @@ class FindReferencesFeature extends GoToDefinitionFeature {
9393
definition.location!,
9494
);
9595

96-
if (!context.includeDeclaration && candidateIsDefinition) {
97-
continue;
98-
} else if (candidateIsDefinition) {
96+
if (candidateIsDefinition) {
9997
references.add(candidate);
10098
continue;
10199
}

‎pkgs/sass_language_services/lib/src/features/find_references/find_references_visitor.dart

Lines changed: 15 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -41,36 +41,17 @@ class FindReferencesVisitor
4141
super.visitExtendRule(node);
4242
}
4343

44-
lsp.Range _getForwardVisibilityRange(sass.ForwardRule node, String name) {
45-
var nameIndex = node.span.text.indexOf(
46-
name,
47-
node.span.start.offset - node.urlSpan.end.offset,
48-
);
49-
50-
var selectionRange = lsp.Range(
51-
start: lsp.Position(
52-
line: node.span.start.line,
53-
character: node.span.start.column + nameIndex,
54-
),
55-
end: lsp.Position(
56-
line: node.span.start.line,
57-
character: node.span.start.column + nameIndex + name.length,
58-
),
59-
);
60-
return selectionRange;
61-
}
62-
6344
@override
6445
void visitForwardRule(sass.ForwardRule node) {
65-
// TODO: would be nice to have span information for forward visibility from sass_api.
46+
// 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.
6647

6748
if (node.hiddenMixinsAndFunctions case var hiddenMixinsAndFunctions?) {
6849
for (var name in hiddenMixinsAndFunctions) {
6950
if (!name.contains(_name)) {
7051
continue;
7152
}
7253

73-
var selectionRange = _getForwardVisibilityRange(node, name);
54+
var selectionRange = forwardVisibilityRange(node, name);
7455
var location = lsp.Location(range: selectionRange, uri: _document.uri);
7556

7657
// We can't tell if this is a mixin or a function, so add a candidate for both.
@@ -97,7 +78,7 @@ class FindReferencesVisitor
9778
continue;
9879
}
9980

100-
var selectionRange = _getForwardVisibilityRange(node, name);
81+
var selectionRange = forwardVisibilityRange(node, '\$$name');
10182
var location = lsp.Location(range: selectionRange, uri: _document.uri);
10283

10384
candidates.add(
@@ -116,7 +97,7 @@ class FindReferencesVisitor
11697
continue;
11798
}
11899

119-
var selectionRange = _getForwardVisibilityRange(node, name);
100+
var selectionRange = forwardVisibilityRange(node, name);
120101
var location = lsp.Location(range: selectionRange, uri: _document.uri);
121102

122103
// We can't tell if this is a mixin or a function, so add a candidate for both.
@@ -143,7 +124,7 @@ class FindReferencesVisitor
143124
continue;
144125
}
145126

146-
var selectionRange = _getForwardVisibilityRange(node, name);
127+
var selectionRange = forwardVisibilityRange(node, '\$$name');
147128
var location = lsp.Location(range: selectionRange, uri: _document.uri);
148129

149130
candidates.add(
@@ -163,6 +144,7 @@ class FindReferencesVisitor
163144
void visitFunctionExpression(sass.FunctionExpression node) {
164145
var name = node.name;
165146
if (!name.contains(_name)) {
147+
super.visitFunctionExpression(node);
166148
return;
167149
}
168150
var location = lsp.Location(
@@ -182,10 +164,12 @@ class FindReferencesVisitor
182164
@override
183165
void visitFunctionRule(sass.FunctionRule node) {
184166
if (!_includeDeclaration) {
167+
super.visitFunctionRule(node);
185168
return;
186169
}
187170
var name = node.name;
188171
if (!name.contains(_name)) {
172+
super.visitFunctionRule(node);
189173
return;
190174
}
191175
var location = lsp.Location(
@@ -206,6 +190,7 @@ class FindReferencesVisitor
206190
void visitIncludeRule(sass.IncludeRule node) {
207191
var name = node.name;
208192
if (!name.contains(_name)) {
193+
super.visitIncludeRule(node);
209194
return;
210195
}
211196
var location = lsp.Location(
@@ -225,10 +210,12 @@ class FindReferencesVisitor
225210
@override
226211
void visitMixinRule(sass.MixinRule node) {
227212
if (!_includeDeclaration) {
213+
super.visitMixinRule(node);
228214
return;
229215
}
230216
var name = node.name;
231217
if (!name.contains(_name)) {
218+
super.visitMixinRule(node);
232219
return;
233220
}
234221
var location = lsp.Location(
@@ -248,6 +235,7 @@ class FindReferencesVisitor
248235
@override
249236
void visitStyleRule(sass.StyleRule node) {
250237
if (!_includeDeclaration) {
238+
super.visitStyleRule(node);
251239
return;
252240
}
253241

@@ -289,10 +277,12 @@ class FindReferencesVisitor
289277
@override
290278
void visitVariableDeclaration(sass.VariableDeclaration node) {
291279
if (!_includeDeclaration) {
280+
super.visitVariableDeclaration(node);
292281
return;
293282
}
294283
var name = node.name;
295284
if (!name.contains(_name)) {
285+
super.visitVariableDeclaration(node);
296286
return;
297287
}
298288
var location = lsp.Location(
@@ -313,6 +303,7 @@ class FindReferencesVisitor
313303
void visitVariableExpression(sass.VariableExpression node) {
314304
var name = node.name;
315305
if (!name.contains(_name)) {
306+
super.visitVariableExpression(node);
316307
return;
317308
}
318309
var location = lsp.Location(

‎pkgs/sass_language_services/lib/src/features/go_to_definition/go_to_definition_feature.dart

Lines changed: 114 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
import 'package:lsp_server/lsp_server.dart' as lsp;
22
import 'package:sass_api/sass_api.dart' as sass;
33
import 'package:sass_language_services/sass_language_services.dart';
4+
import 'package:sass_language_services/src/features/find_references/reference.dart';
45
import 'package:sass_language_services/src/features/go_to_definition/scoped_symbols.dart';
56
import 'package:sass_language_services/src/features/node_at_offset_visitor.dart';
67

8+
import '../../utils/sass_lsp_utils.dart';
79
import '../language_feature.dart';
810
import 'definition.dart';
911
import 'scope_visitor.dart';
@@ -34,30 +36,51 @@ class GoToDefinitionFeature extends LanguageFeature {
3436
return null;
3537
}
3638

37-
// Get the node's ReferenceKind and name so we can compare it to other symbols.
38-
var kind = getNodeReferenceKind(node);
39-
if (kind == null) {
39+
// The visibility configuration needs special handling.
40+
// We don't always know if something refers to a function or mixin, so
41+
// we check for both kinds. Only relevant for the workspace traversal though.
42+
String? name;
43+
var kinds = <ReferenceKind>[];
44+
if (node is sass.ForwardRule) {
45+
var result = _getForwardVisibilityCandidates(node, position);
46+
if (result != null) {
47+
(name, kinds) = result;
48+
}
49+
} else {
50+
// Get the node's ReferenceKind and name so we can compare it to other symbols.
51+
var kind = getNodeReferenceKind(node);
52+
if (kind == null) {
53+
return null;
54+
}
55+
kinds = [kind];
56+
57+
name = getNodeName(node);
58+
if (name == null) {
59+
return null;
60+
}
61+
62+
// Look for the symbol in the current document.
63+
// It may be a scoped symbol.
64+
var symbols = ScopedSymbols(stylesheet,
65+
document.languageId == 'sass' ? Dialect.indented : Dialect.scss);
66+
var symbol = symbols.findSymbolFromNode(node);
67+
if (symbol != null) {
68+
// Found the definition in the same document.
69+
return Definition(
70+
name,
71+
kind,
72+
lsp.Location(uri: document.uri, range: symbol.selectionRange),
73+
);
74+
}
75+
}
76+
77+
if (kinds.isEmpty) {
4078
return null;
4179
}
42-
var name = getNodeName(node);
4380
if (name == null) {
4481
return null;
4582
}
4683

47-
// Look for the symbol in the current document.
48-
// It may be a scoped symbol.
49-
var symbols = ScopedSymbols(stylesheet,
50-
document.languageId == 'sass' ? Dialect.indented : Dialect.scss);
51-
var symbol = symbols.findSymbolFromNode(node);
52-
if (symbol != null) {
53-
// Found the definition in the same document.
54-
return Definition(
55-
name,
56-
kind,
57-
lsp.Location(uri: document.uri, range: symbol.selectionRange),
58-
);
59-
}
60-
6184
// Start looking from the linked document In case of a namespace
6285
// so we don't accidentally match with a symbol of the same kind
6386
// and name, but in a different module.
@@ -96,31 +119,32 @@ class GoToDefinitionFeature extends LanguageFeature {
96119
required List<String> shownMixinsAndFunctions,
97120
required List<String> shownVariables,
98121
}) async {
99-
// `@forward` may add a prefix to [name],
100-
// but we're comparing it to symbols without that prefix.
101-
var unprefixedName = kind == ReferenceKind.function ||
102-
kind == ReferenceKind.mixin ||
103-
kind == ReferenceKind.variable
104-
? name.replaceFirst(prefix, '')
105-
: name;
106-
107-
var stylesheet = ls.parseStylesheet(document);
108-
var symbols = ScopedSymbols(stylesheet,
109-
document.languageId == 'sass' ? Dialect.indented : Dialect.scss);
110-
var symbol = symbols.globalScope.getSymbol(
111-
name: unprefixedName,
112-
referenceKind: kind,
113-
);
122+
for (var kind in kinds) {
123+
// `@forward` may add a prefix to [name],
124+
// but we're comparing it to symbols without that prefix.
125+
var unprefixedName = kind == ReferenceKind.function ||
126+
kind == ReferenceKind.mixin ||
127+
kind == ReferenceKind.variable
128+
? name!.replaceFirst(prefix, '')
129+
: name!;
130+
131+
var stylesheet = ls.parseStylesheet(document);
132+
var symbols = ScopedSymbols(stylesheet,
133+
document.languageId == 'sass' ? Dialect.indented : Dialect.scss);
134+
var symbol = symbols.globalScope.getSymbol(
135+
name: unprefixedName,
136+
referenceKind: kind,
137+
);
114138

115-
if (symbol != null) {
116-
return [
117-
(
118-
symbol,
119-
lsp.Location(uri: document.uri, range: symbol.selectionRange)
120-
)
121-
];
139+
if (symbol != null) {
140+
return [
141+
(
142+
symbol,
143+
lsp.Location(uri: document.uri, range: symbol.selectionRange)
144+
)
145+
];
146+
}
122147
}
123-
124148
return null;
125149
},
126150
);
@@ -139,17 +163,59 @@ class GoToDefinitionFeature extends LanguageFeature {
139163
for (var document in ls.cache.getDocuments()) {
140164
var symbols = ls.findDocumentSymbols(document);
141165
for (var symbol in symbols) {
142-
if (symbol.name == name && symbol.referenceKind == kind) {
143-
return Definition(
144-
name,
145-
kind,
146-
lsp.Location(uri: document.uri, range: symbol.selectionRange),
147-
);
166+
for (var kind in kinds) {
167+
if (symbol.name == name && symbol.referenceKind == kind) {
168+
return Definition(
169+
name,
170+
kind,
171+
lsp.Location(uri: document.uri, range: symbol.selectionRange),
172+
);
173+
}
148174
}
149175
}
150176
}
151177

152178
// Could be a Sass built-in module.
153-
return Definition(name, kind, null);
179+
return Definition(name, kinds.first, null);
180+
}
181+
182+
(String, List<ReferenceKind>)? _getForwardVisibilityCandidates(
183+
sass.ForwardRule node, lsp.Position position) {
184+
if (node.hiddenMixinsAndFunctions case var hiddenMixinsAndFunctions?) {
185+
for (var name in hiddenMixinsAndFunctions) {
186+
var selectionRange = forwardVisibilityRange(node, name);
187+
if (isInRange(position: position, range: selectionRange)) {
188+
return (name, [ReferenceKind.function, ReferenceKind.mixin]);
189+
}
190+
}
191+
}
192+
193+
if (node.hiddenVariables case var hiddenVariables?) {
194+
for (var name in hiddenVariables) {
195+
var selectionRange = forwardVisibilityRange(node, '\$$name');
196+
if (isInRange(position: position, range: selectionRange)) {
197+
return (name, [ReferenceKind.variable]);
198+
}
199+
}
200+
}
201+
202+
if (node.shownMixinsAndFunctions case var shownMixinsAndFunctions?) {
203+
for (var name in shownMixinsAndFunctions) {
204+
var selectionRange = forwardVisibilityRange(node, name);
205+
if (isInRange(position: position, range: selectionRange)) {
206+
return (name, [ReferenceKind.function, ReferenceKind.mixin]);
207+
}
208+
}
209+
}
210+
211+
if (node.shownVariables case var shownVariables?) {
212+
for (var name in shownVariables) {
213+
var selectionRange = forwardVisibilityRange(node, '\$$name');
214+
if (isInRange(position: position, range: selectionRange)) {
215+
return (name, [ReferenceKind.variable]);
216+
}
217+
}
218+
}
219+
return null;
154220
}
155221
}

‎pkgs/sass_language_services/lib/src/utils/sass_lsp_utils.dart

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,3 +25,29 @@ lsp.Range selectorNameRange(
2525
),
2626
);
2727
}
28+
29+
lsp.Range forwardVisibilityRange(sass.ForwardRule node, String name) {
30+
var nameIndex = node.span.text.indexOf(
31+
name,
32+
node.span.start.offset + node.urlSpan.end.offset,
33+
);
34+
35+
var selectionRange = lsp.Range(
36+
start: lsp.Position(
37+
line: node.span.start.line,
38+
character: node.span.start.column + nameIndex,
39+
),
40+
end: lsp.Position(
41+
line: node.span.start.line,
42+
character: node.span.start.column + nameIndex + name.length,
43+
),
44+
);
45+
return selectionRange;
46+
}
47+
48+
bool isInRange({required lsp.Position position, required lsp.Range range}) {
49+
return range.start.line <= position.line &&
50+
range.start.character <= position.character &&
51+
range.end.line >= position.line &&
52+
range.end.character >= position.character;
53+
}

0 commit comments

Comments
 (0)