Skip to content

Commit 0e45af6

Browse files
Copilotnixel2007
andcommitted
Address code review feedback - improve comments and code quality
Fixed several issues identified in automated code review: 1. Fixed comment numbering: Changed "6) Build delta-encoded data" to "7)" as it's the 7th step 2. Improved comment accuracy for SemanticTokenTypes.Type - clarified it's for type names (identifiers of types) 3. Updated comment for SemanticTokenTypes.Class - clarified it's for metadata object names (e.g., Контрагенты, Валюты), not metadata types 4. Improved ambiguous identifier comment in visitColumn - explained single identifiers are intentionally treated as variables because distinguishing requires symbol resolution not performed in visitor 5. Clarified newline handling comment in test - explained that merged multiline token length is sum of lines without newlines 6. Better explained emptyRef variable name - matches grammar field 'emptyFer' (typo in grammar for 'emptyRef') Note: Did NOT remove SemanticTokenTypes.Type from legend as it may be used by future extensions Note: Did NOT extract duplicate SDBL token collection code as contexts differ (collection vs processing) Note: Did NOT change multi-line range handling as SDBL contexts are always single-line Note: Did NOT change middle identifier handling as SDBL doesn't support 3-part column references All changes improve code documentation and clarity without changing functionality. All 27 tests passing. Co-authored-by: nixel2007 <[email protected]>
1 parent 952e789 commit 0e45af6

File tree

3 files changed

+11
-8
lines changed

3 files changed

+11
-8
lines changed

src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ public SemanticTokens getSemanticTokensFull(DocumentContext documentContext, @Su
241241
// 6) Add SDBL tokens and split string parts
242242
addSdblTokens(documentContext, entries, stringsToSkip);
243243

244-
// 6) Build delta-encoded data
244+
// 7) Build delta-encoded data
245245
List<Integer> data = toDeltaEncoded(entries);
246246
return new SemanticTokens(data);
247247
}
@@ -1131,8 +1131,10 @@ public Void visitColumn(SDBLParser.ColumnContext ctx) {
11311131
var identifiers = ctx.identifier();
11321132
if (identifiers != null && !identifiers.isEmpty()) {
11331133
if (identifiers.size() == 1) {
1134-
// Single identifier - could be alias or field
1135-
// Context-dependent, treat as variable for now
1134+
// Single identifier: in SDBL it may represent either a table alias or a field name.
1135+
// We intentionally highlight such ambiguous identifiers as "variable" for now,
1136+
// because distinguishing alias vs. field here would require deeper symbol resolution
1137+
// that is not performed in this visitor.
11361138
provider.addSdblTokenRange(entries, identifiers.get(0).getStart(), SemanticTokenTypes.Variable);
11371139
} else if (identifiers.size() >= 2) {
11381140
// First identifier → Variable (table alias)
@@ -1175,7 +1177,7 @@ public Void visitValueFunction(SDBLParser.ValueFunctionContext ctx) {
11751177
var type = ctx.type;
11761178
var mdoName = ctx.mdoName;
11771179
var predefinedName = ctx.predefinedName;
1178-
var emptyRef = ctx.emptyFer; // Note: grammar has typo "emptyFer" instead of "emptyRef"
1180+
var emptyRef = ctx.emptyFer; // Note: variable name matches grammar field 'emptyFer' (typo in grammar for 'emptyRef')
11791181
var systemName = ctx.systemName;
11801182

11811183
if (type != null && mdoName != null) {

src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/SemanticTokensLegendConfiguration.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ public SemanticTokensLegend semanticTokensLegend() {
5959
SemanticTokenTypes.Decorator,
6060
SemanticTokenTypes.Operator,
6161
SemanticTokenTypes.Namespace,
62-
SemanticTokenTypes.Type, // Added for general type references
62+
SemanticTokenTypes.Type, // Standard LSP token type for type names (identifiers of types)
6363
SemanticTokenTypes.Property, // Added for SDBL field names
64-
SemanticTokenTypes.Class, // Added for SDBL metadata types (Справочник, РегистрСведений, etc.)
64+
SemanticTokenTypes.Class, // Added for SDBL metadata object names (e.g. Справочник.Контрагенты, РегистрСведений.КурсыВалют)
6565
SemanticTokenTypes.Enum, // Added for SDBL enum types (Перечисление.Пол)
6666
SemanticTokenTypes.EnumMember // Added for predefined elements and enum values
6767
);

src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -437,10 +437,11 @@ void multilineDocumentation_mergedWhenSupported() {
437437

438438
// When multiline support is enabled, documentation comments should be merged into one token
439439
// The merged token starts on line 0 and spans across lines
440-
// Both lines "// Первая строка описания" (26 chars) + "// Вторая строка описания" (25 chars) = 51 chars total
440+
// Both lines "// Первая строка описания" (26 chars) + "// Вторая строка описания" (25 chars) = 51 chars total,
441+
// i.e. the sum of the characters of both lines; the newline between them is not included in the length.
441442
// Body comment on line 3 should NOT have Documentation modifier
442443
var expected = List.of(
443-
// Merged documentation comment (starts at line 0, length is sum of both lines)
444+
// Merged documentation comment (starts at line 0, length is sum of both lines without the newline)
444445
new ExpectedToken(0, 0, 51, SemanticTokenTypes.Comment, SemanticTokenModifiers.Documentation, "// Первая+Вторая строка описания"),
445446
// Body comment without documentation modifier
446447
new ExpectedToken(3, 2, 18, SemanticTokenTypes.Comment, "// не документация")

0 commit comments

Comments
 (0)