Skip to content

Commit 57603d2

Browse files
nixel2007nixel2007
authored
Merge pull request #1310 from 1c-syntax/feature/fixVariableSymbolDescription
Исправлен алгоритм определения описания у переменной
2 parents 0fd4665 + 658a57f commit 57603d2

File tree

7 files changed

+144
-38
lines changed

7 files changed

+144
-38
lines changed

‎src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java‎

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,21 @@ private VariableSymbol createVariableSymbol(
8282
.variableNameRange(Ranges.create(varName))
8383
.export(export)
8484
.kind(kind)
85-
.description(createDescription(getTokenToSearchComments(ctx)))
85+
.description(createDescription(ctx))
8686
.build();
8787
}
8888

89-
private Optional<VariableDescription> createDescription(Token token) {
89+
private Optional<VariableDescription> createDescription(BSLParserRuleContext ctx) {
9090
List<Token> tokens = documentContext.getTokens();
91-
List<Token> comments = Trees.getComments(tokens, token);
92-
Optional<Token> trailingComments = Trees.getTrailingComment(tokens, token);
91+
List<Token> comments = new ArrayList<>();
92+
93+
// поиск комментариев начинается от первого токена - VAR
94+
var varToken = Trees.getPreviousTokenFromDefaultChannel(tokens,
95+
ctx.getStart().getTokenIndex(), BSLParser.VAR_KEYWORD);
96+
varToken.ifPresent(value -> comments.addAll(Trees.getComments(tokens, value)));
97+
98+
// висячий комментарий смотрим по токену переменной, он должен находится в этой же строке
99+
Optional<Token> trailingComments = Trees.getTrailingComment(tokens, ctx.getStop());
93100

94101
if (comments.isEmpty() && trailingComments.isEmpty()) {
95102
return Optional.empty();
@@ -114,14 +121,6 @@ private Optional<VariableDescription> createDescription(Token token) {
114121
return Optional.of(description);
115122
}
116123

117-
private static Token getTokenToSearchComments(BSLParserRuleContext declaration) {
118-
var parent = Trees.getAncestorByRuleIndex(declaration, BSLParser.RULE_moduleVar);
119-
if (parent == null) {
120-
return declaration.getStart();
121-
}
122-
return parent.getStart();
123-
}
124-
125124
private static Range getRangeForDescription(List<Token> tokens) {
126125

127126
if (tokens.isEmpty()) {

‎src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,18 @@ public static Range create(int startLine, int startChar, int endLine, int endCha
3737
return new Range(new Position(startLine, startChar), new Position(endLine, endChar));
3838
}
3939

40+
/**
41+
* Создание Range для линии
42+
*
43+
* @param lineNo - номер строки
44+
* @param startChar - номер первого символа
45+
* @param endChar - номер последнего символа
46+
* @return - полученный Range
47+
*/
48+
public static Range create(int lineNo, int startChar, int endChar) {
49+
return new Range(new Position(lineNo, startChar), new Position(lineNo, endChar));
50+
}
51+
4052
public static Range create(ParserRuleContext ruleContext) {
4153
return create(ruleContext.getStart(), ruleContext.getStop());
4254
}

‎src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,28 @@ public static ParseTree getPreviousNode(ParseTree parent, ParseTree tnc, int rul
171171
return tnc;
172172
}
173173

174+
/**
175+
* @param tokens - полный список токенов (см. {@link com.github._1c_syntax.bsl.languageserver.context.DocumentContext#getTokens()}
176+
* @param tokenIndex - индекс текущего токена в переданном списке токенов
177+
* @param tokenType - тип искомого токена (см. {@link com.github._1c_syntax.bsl.parser.BSLParser}
178+
* @return предыдущий токен, если он был найден
179+
*/
180+
public Optional<Token> getPreviousTokenFromDefaultChannel(List<Token> tokens, int tokenIndex, int tokenType) {
181+
while (true) {
182+
if (tokenIndex == 0) {
183+
return Optional.empty();
184+
}
185+
Token token = tokens.get(tokenIndex);
186+
if (token.getChannel() != Token.DEFAULT_CHANNEL
187+
|| token.getType() != tokenType) {
188+
tokenIndex = tokenIndex - 1;
189+
continue;
190+
}
191+
192+
return Optional.of(token);
193+
}
194+
}
195+
174196
/**
175197
* @param tokens - полный список токенов (см. {@link com.github._1c_syntax.bsl.languageserver.context.DocumentContext#getTokens()}
176198
* @param tokenIndex - индекс текущего токена в переданном списке токенов
@@ -413,7 +435,6 @@ private static boolean isBlankLine(Token previousToken, Token currentToken) {
413435
|| (previousToken.getLine() + 1) != currentToken.getLine());
414436
}
415437

416-
417438
private static boolean treeContainsErrors(ParseTree tnc, boolean recursive) {
418439
if (!(tnc instanceof BSLParserRuleContext)) {
419440
return false;

‎src/test/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolTest.java‎

Lines changed: 33 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -52,24 +52,29 @@ void setup() {
5252
@Test
5353
void testVariableSymbolDescription() {
5454

55-
assertThat(variableSymbols).hasSize(8);
55+
assertThat(variableSymbols).hasSize(13);
5656

5757
assertThat(variableSymbols)
5858
.filteredOn(variableSymbol -> variableSymbol.getDescription().isEmpty())
5959
.hasSize(5)
60-
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(12, 6, 12, 34)))
61-
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(14, 6, 14, 27)))
62-
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(16, 6, 16, 17)))
63-
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(16, 19, 16, 30)))
64-
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(19, 10, 19, 19)))
60+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(12, 6, 34)))
61+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(14, 6, 27)))
62+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(16, 6, 17)))
63+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(16, 19, 30)))
64+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(27, 10, 19)))
6565
;
6666

6767
assertThat(variableSymbols)
6868
.filteredOn(variableSymbol -> variableSymbol.getDescription().isPresent())
69-
.hasSize(3)
70-
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(2, 6, 2, 32)))
71-
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(6, 6, 6, 32)))
72-
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(8, 6, 8, 33)))
69+
.hasSize(8)
70+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(2, 6, 32)))
71+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(6, 6, 32)))
72+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(8, 6, 33)))
73+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(19, 6, 18)))
74+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(24, 6, 18)))
75+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(29, 10, 20)))
76+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(33, 10, 20)))
77+
.anyMatch(variableSymbol -> variableSymbol.getRange().equals(Ranges.create(40, 10, 21)))
7378
;
7479

7580
}
@@ -83,21 +88,27 @@ void testVariableDescriptionRange() {
8388
.map(Optional::get)
8489
.collect(Collectors.toList());
8590

86-
assertThat(variableDescriptions).hasSize(3);
87-
8891
assertThat(variableDescriptions)
92+
.hasSize(8)
8993
.filteredOn(variableDescription -> !variableDescription.getDescription().equals(""))
90-
.hasSize(2)
91-
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(1, 0, 1, 18)))
94+
.hasSize(5)
95+
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(1, 0, 18)))
9296
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(4, 0, 5, 23)))
97+
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(21, 0, 23, 29)))
98+
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(31, 4, 25)))
99+
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(35, 4, 39, 27)))
93100
;
94101

95102
assertThat(variableDescriptions)
96103
.extracting(VariableDescription::getTrailingDescription)
97104
.filteredOn(Optional::isPresent)
98-
.hasSize(1)
105+
.hasSize(5)
99106
.extracting(Optional::get)
100-
.anyMatch(trailingDescription -> trailingDescription.getRange().equals(Ranges.create(8, 35, 8, 55)))
107+
.anyMatch(trailingDescription -> trailingDescription.getRange().equals(Ranges.create(8, 35, 55)))
108+
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(19, 20, 42)))
109+
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(24, 20, 42)))
110+
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(29, 21, 43)))
111+
.anyMatch(variableDescription -> variableDescription.getRange().equals(Ranges.create(33, 21, 43)))
101112
;
102113

103114
}
@@ -108,20 +119,19 @@ void testVariableNameRange() {
108119
assertThat(variableSymbols)
109120
.filteredOn(variableSymbol -> variableSymbol.getDescription().isEmpty())
110121
.hasSize(5)
111-
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(12, 6, 12, 34)))
112-
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(14, 6, 14, 27)))
113-
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(16, 6, 16, 17)))
114-
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(16, 19, 16, 30)))
115-
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(19, 10, 19, 19)))
116-
122+
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(12, 6, 34)))
123+
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(14, 6, 27)))
124+
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(16, 6, 17)))
125+
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(16, 19, 30)))
126+
.anyMatch(variableName -> variableName.getVariableNameRange().equals(Ranges.create(27, 10, 19)))
117127
;
118128
}
119129

120130
@Test
121131
void testVariableKind() {
122132

123133
assertThat(variableSymbols.get(0).getKind()).isEqualTo(VariableKind.MODULE);
124-
assertThat(variableSymbols.get(7).getKind()).isEqualTo(VariableKind.LOCAL);
134+
assertThat(variableSymbols.get(variableSymbols.size() - 1).getKind()).isEqualTo(VariableKind.LOCAL);
125135

126136
}
127137

‎src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingVariablesDescriptionDiagnosticTest.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,13 @@ void test() {
3838

3939
List<Diagnostic> diagnostics = getDiagnostics();
4040

41-
assertThat(diagnostics).hasSize(4);
41+
assertThat(diagnostics).hasSize(5);
4242
assertThat(diagnostics, true)
4343
.hasRange(1, 6, 1, 27)
4444
.hasRange(3, 6, 3, 45)
4545
.hasRange(17, 6, 17, 38)
46-
.hasRange(21, 6, 21, 56);
46+
.hasRange(21, 6, 21, 56)
47+
.hasRange(37, 6, 49);
4748

4849
}
4950
}

‎src/test/resources/context/symbol/variableSymbolTest.bsl‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,28 @@
1616

1717
Перем ПеременнаяА, ПеременнаяБ;
1818

19+
&НаКлиенте
20+
Перем ПеременнаяАф; // висячий комментарий
21+
22+
// Комментарий сверху
23+
&НаКлиенте
24+
// Комментарий под аннотацией
25+
Перем ПеременнаяАя; // висячий комментарий
26+
1927
Процедура А()
2028
Перем Локальная;
2129

30+
Перем Локальнаяв;// висячий комментарий
31+
32+
// комментарий сверху
33+
&НаКлиенте
34+
Перем Локальнаяя;// висячий комментарий
35+
36+
// комментарий сверху
37+
// комментарий сверху 2
38+
&НаКлиенте
39+
// комментарий сверху 3
40+
// комментарий сверху 4
41+
Перем Локальнаяяя;
42+
2243
КонецПроцедуры

‎src/test/resources/diagnostics/MissingVariablesDescriptionDiagnostic.bsl‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,45 @@
2020
// неточное описание
2121

2222
Перем ЭкспортнаяПеременнаяСНевернымОписаниемВыше Экспорт;
23+
24+
&Идентификатор
25+
&ГенерируемоеЗначение
26+
&Колонка(Тип = "Целое")
27+
Перем ПеременнаяСАннотациямиOS Экспорт; // Внутренний идентификатор объекта
28+
29+
// Описание какое-то в шапке
30+
&Идентификатор
31+
&ГенерируемоеЗначение
32+
&Колонка(Тип = "НеЦелое")
33+
Перем ПеременнаяСАннотациямиOS Экспорт; // Висячий комментарий
34+
35+
&Идентификатор
36+
&ГенерируемоеЗначение
37+
&Колонка(Тип = "Вместилище")
38+
Перем ПеременнаяСАннотациямиOSБезОписания Экспорт;
39+
40+
&Идентификатор
41+
&ГенерируемоеЗначение
42+
Перем ПеременнаяСАннотациямиСОписаниемВСтроке Экспорт; // это описание
43+
44+
// это описание переменной в шапке
45+
&Идентификатор
46+
&ГенерируемоеЗначение
47+
Перем ПеременнаяСАннотациямиСОписаниемВШапке Экспорт;
48+
49+
&Идентификатор
50+
// это описание переменной между аннотациями, но тоже подойдет
51+
&ГенерируемоеЗначение
52+
Перем ПеременнаяСАннотациямиСОписаниемВШапке Экспорт;
53+
54+
&Идентификатор
55+
&ГенерируемоеЗначение
56+
// это описание переменной под аннотациями
57+
Перем ПеременнаяСАннотациямиСОписаниемВШапке Экспорт;
58+
59+
// это описание переменной в шапке и будет использовано именно оно
60+
&Идентификатор
61+
// это описание переменной между аннотациями использовано не будет
62+
&ГенерируемоеЗначение
63+
// это описание переменной под аннотациями и так как есть в шапке, будем игнорировать
64+
Перем ПеременнаяСАннотациямиСОписаниемВШапке;

0 commit comments

Comments
 (0)