Исправление замечаний к коду, рефакторинг #3657
Conversation
WalkthroughApplied a widespread nullability sweep and minor refactors: added Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Areas needing extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/variable/VariableDescription.java (1)
100-108: Missing null safety after removing@NonNullannotation.The removal of
@NonNullfrom theparamparameter (line 100) eliminates compile-time null safety, yet line 106 dereferencesparamwithparam.link()without any null check. Ifparamis null, this will throw aNullPointerException.Choose one of the following solutions:
Solution 1: Add explicit null check (if param can be null):
public VariableDescription(ParameterDescription param) { + if (param == null) { + throw new IllegalArgumentException("param cannot be null"); + } description = "";Solution 2: Use
Objects.requireNonNull(if param must not be null):+import static java.util.Objects.requireNonNull; + public VariableDescription(ParameterDescription param) { + requireNonNull(param, "param cannot be null"); description = "";Solution 3: If migrating to JSpecify, add the appropriate annotation:
+import org.jspecify.annotations.NonNull; + -public VariableDescription(ParameterDescription param) { +public VariableDescription(@NonNull ParameterDescription param) {Note: The unused
lombok.NonNullimport on line 29 should also be removed if no longer needed.src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/RenameProvider.java (1)
102-107: Update JavaDoc to document null return case.The
@Nullableannotation correctly reflects that this method can return null (line 106), improving null-safety metadata. However, the JavaDoc (lines 95-101) currently states@return Rangewithout documenting when null is returned, which should be clarified for API consumers.Update the JavaDoc to:
/** * {@link Range} * * @param documentContext Контекст документа. * @param params Параметры вызова. - * @return Range + * @return Range для переименования или null, если позиция не указывает на переименовываемый символ */src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java (1)
80-88: Комментарий вget(...)противоречит условиюСейчас условие
if (value != null)возвращает значение из кэша, но комментарий говорит «если нет в кеше, загружаем из valueLoader». Это вводит в заблуждение.Предлагаю либо поправить комментарий, либо перенести его к ветке, где действительно вызывается
valueLoader:- if (value != null) { // если нет в кеше, загружаем из valueLoader - return (T) value; - } + if (value != null) { // значение найдено в кэше — возвращаем его + return (T) value; + }
🧹 Nitpick comments (15)
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java (1)
150-153: Minor indentation inconsistency.The second parameter
BSLParser.RULE_annotationParamValueis aligned to the start of the line rather than to the opening parenthesis or the first parameter. Consider aligning consistently.var annotationParamValueContext = parentContext instanceof BSLParser.AnnotationParamValueContext apv ? apv : Trees.<BSLParser.AnnotationParamValueContext>getAncestorByRuleIndex( - parentContext, BSLParser.RULE_annotationParamValue); + parentContext, BSLParser.RULE_annotationParamValue);src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java (1)
114-139: Good extraction! Consider returning interface type.The helper method successfully consolidates flag checks, improving modularity and testability. However, returning
ArrayList<String>is more specific than necessary.Apply this diff to return the interface type:
- private ArrayList<String> getModuleFlags(CommonModule commonModule) { + private List<String> getModuleFlags(CommonModule commonModule) { var flags = new ArrayList<String>();Add this import at the top:
import java.util.List;src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java (1)
116-134: Refactoring improves readability.The consolidation of logic under a single
expression.isPresent()guard is cleaner and preserves the original behavior correctly.Minor simplification opportunity: Lines 129-131 use
expression.filter(...).isPresent()within an already-confirmedexpression.isPresent()block. This could be simplified to directly evaluate the predicate:!isNumericExpression(expression.get()) || insideCallParam(expression.get()), but the current approach is also acceptable.src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java (1)
104-116:instanceofpattern matching & Preprocessor handling look good; consider clarifying the fallback exceptionThe switch to
instanceofpattern matching forOperationContext/MemberContextand explicit skipping ofPreprocessorContextmakes the dispatch clearer and removes spurious failures for valid preprocessor nodes. The remainingelsethat throwsnew IllegalStateException()is fine as a safety net, but you might want to include the unexpected node’s class in the message to ease debugging (e.g."Unexpected child in ExpressionContext: " + child.getClass()).src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CommandProvider.java (1)
59-81: The@Nullableannotation accurately reflects the method behavior.The method can return
null(line 69:.orElse(null)), making the annotation correct and beneficial for null-safety. However, verify that all callers properly handle the null return value.Run the following script to identify callers and verify null handling:
#!/bin/bash # Description: Find all callers of executeCommand method and verify null handling # Search for calls to executeCommand in the codebase rg -n -P --type=java -C5 'executeCommand\s*\(' | rg -v 'public.*executeCommand'Optional: Update JavaDoc to document null return.
Consider updating the JavaDoc (lines 52-58) to explicitly mention that this method may return
null.src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java (2)
311-333: Excellent refactoring: consolidated logic and improved null safety.The extraction of
addAmpersandRangeeliminates duplicate code for handling compiler directives and annotations. The explicit null check forannotationParamsat line 322 is good defensive programming that prevents potential NPE when annotations lack parameters.
423-432: Well-designed helper method with proper null handling.The method correctly consolidates the ampersand range logic for both annotations and compiler directives. The
@Nullableannotation on thenameparameter is appropriate, and the null handling is explicit and correct.Consider adding a brief JavaDoc comment to describe the method's purpose, even though it's private:
+ /** + * Adds a decorator range starting from ampersand through the optional name token. + * Used for both compiler directives and annotations. + * + * @param entries token entries list + * @param node ampersand terminal node + * @param name optional name context (compiler directive symbol or annotation name) + */ // общий для аннотаций и директив компиляции способ добавления private void addAmpersandRange(List<TokenEntry> entries, TerminalNode node, @Nullable ParserRuleContext name) {src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java (1)
72-79: Nullability annotation onexecutecorrectly reflects existing behaviorAnnotating the return type as
@Nullablematches the current logic (returningnullto drop the event) without changing runtime behavior and keeps the override compatible with Sentry’sBeforeSendCallback. Dropping Lombok@NonNullon parameters is also consistent with moving nullness handling to jspecify-level defaults while still using Lombok only for boilerplate-reduction annotations.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.java (1)
73-88: Clarify cube vs dimension table diagnostics and tighten null‑filteringThe new branch correctly adds a diagnostic for missing cube dimension tables, but with the current logic you’ll emit two diagnostics when the cube itself is missing:
nonCubeExists(eds)istrue→ diagnostic oneds.cubeName.- For the same
eds,getCubeDimTable(eds)also returns empty (no cube → no dimension tables) →nonCubeDimTableExists(eds)istrue→ second diagnostic oneds.tableName.If that double signal is not desired, consider checking the dimension table only when the cube exists, and pushing the
eds.tableName != nullguard into the stream filters to keep null‑checks in one place and avoid the extra local variable:ctx.dataSource().stream() .map(SDBLParser.DataSourceContext::externalDataSourceTable) .filter(Objects::nonNull) .filter(eds -> eds.cubeName != null) .filter(eds -> eds.mdo() != null && eds.mdo().tableName != null) + .filter(eds -> eds.tableName != null) - .forEach((SDBLParser.ExternalDataSourceTableContext eds) -> { - if (nonCubeExists(eds)) { - diagnosticStorage.addDiagnostic(eds.cubeName, info.getMessage(eds.cubeName.getText())); - } - - var cubeDimTable = eds.tableName; - - if (cubeDimTable != null && nonCubeDimTableExists(eds)) { - diagnosticStorage.addDiagnostic(cubeDimTable, info.getMessage(eds.tableName.getText())); - } - }); + .forEach((SDBLParser.ExternalDataSourceTableContext eds) -> { + var cubeMissing = nonCubeExists(eds); + if (cubeMissing) { + diagnosticStorage.addDiagnostic(eds.cubeName, info.getMessage(eds.cubeName.getText())); + } + + if (!cubeMissing && nonCubeDimTableExists(eds)) { + diagnosticStorage.addDiagnostic(eds.tableName, info.getMessage(eds.tableName.getText())); + } + });This keeps diagnostics 1:1 with the actual missing metadata element (cube vs dimension table) and keeps the pipeline’s null assumptions explicit.
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/UTF8Control.java (1)
50-51: LGTM! Type inference usage is clear and idiomatic.The use of
varfor local variables is appropriate and follows modern Java conventions. The inferred types are clear from the right-hand side expressions.For consistency, consider also using
varfor thestreamvariable on line 53:- InputStream stream = null; + var stream = (InputStream) null;However, keeping it as an explicit type is also reasonable since it's assigned in multiple branches.
Also applies to: 55-55, 57-57, 68-68
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.java (1)
123-129: Explicit lambda parameter type is fine; optional style alignmentSwitching to an explicitly typed lambda parameter here is clear and type-safe. Note that
publishDiagnosticsbelow still uses type inference in its lambda; if your style guide prefers consistency, you might consider aligning both call sites (either both explicit or both inferred), but functionally this is OK as-is.src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.java (1)
84-101: Simplify null handling and comparison in the async logging blockThe async refactor looks good, but the inner block can be tightened up:
Objects.requireNonNull(languageClientHolder)is redundant after an explicit null check.- Using a local variable avoids a check‑then‑use pattern.
SentryId.EMPTY_ID.equals(sentryId)is a bit safer ifcaptureExceptionever returnsnull.A minimal refactor could look like:
- CompletableFuture.runAsync(() -> { - var sentryId = Sentry.captureException(ex); - if (sentryId.equals(SentryId.EMPTY_ID)) { - return; - } - if (languageClientHolder == null) { - return; - } + CompletableFuture.runAsync(() -> { + var sentryId = Sentry.captureException(ex); + if (SentryId.EMPTY_ID.equals(sentryId)) { + return; + } + var clientHolder = languageClientHolder; + if (clientHolder == null) { + return; + } var messageType = MessageType.Info; var message = resources.getResourceString(getClass(), "logMessage", sentryId); var messageParams = new MessageParams(messageType, message); - - Objects.requireNonNull(languageClientHolder) - .execIfConnected(languageClient -> languageClient.showMessage(messageParams)); + clientHolder.execIfConnected( + languageClient -> languageClient.showMessage(messageParams) + ); }, executorService);After this change, the
java.util.Objectsimport becomes unused and can be removed.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java (2)
122-136: Optional-based variable type resolution is sound; only minor readability nitsThe refactors to use
Optional.flatMapandfilter(...).isPresent()inisSecondParameterOfStructureInsert,getVariableTypeFromCallParamList,getVariableTypeFromCallStatement, andgetVariableTypeFromASTcorrectly propagate theOptional<Boolean>semantics and avoid unsafeget()calls. If you want to squeeze a bit more readability, you could avoid thefilter(...).isPresent()idiom and express it directly as a boolean, e.g.:return getVariableTypeFromCallParamList(callParamList) .map(typeIsCorrespondence -> !typeIsCorrespondence) .orElse(false);But the current version is logically correct.
Also applies to: 217-224, 253-256, 310-312
489-492: Consider reducing repeated Optional.get calls in structure-constructor helpersIn
checkInsideStructureConstructor,callParamContextOpt.get()is used multiple times, andtypeNameOpt.filter(...).isPresent()is used only to return a boolean. For clarity and to avoid repeatedget()usage, you could cache the context and collapse the chain, e.g.:var callParamContext = callParamContextOpt.get(); var typeNameOpt = getStructureTypeName(callParamContext); return typeNameOpt .map(typeName -> isValidStructureConstructorParameter(callParamContext, typeName)) .orElse(false);This keeps behavior identical while making data-flow a bit easier to follow.
Also applies to: 547-572
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.java (1)
151-175: checkMetadataObject refactor is behavior‑preserving; consider inlining returnsChanging
var result = false;toboolean result;with explicit assignments in each branch keeps the method’s behavior identical and is perfectly fine. If you want to further simplify, you could remove the local variable entirely and return directly from the type branches, e.g.:if (mdoType == MDOType.CATALOG) { return isUnsafeCatalogUsage((Catalog) mdo); } else if (mdoType == MDOType.CHART_OF_CHARACTERISTIC_TYPES) { return isUnsafeChartOfCharacteristicTypesUsage((ChartOfCharacteristicTypes) mdo); } else if (mdoType == MDOType.CHART_OF_ACCOUNTS) { return isUnsafeChartOfAccountsUsage((ChartOfAccounts) mdo); } return false;Purely optional; current code is correct.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (40)
build.gradle.kts(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/ClientCapabilitiesHolder.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/MeasuresAspect.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CognitiveComplexityComputer.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CyclomaticComplexityComputer.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/ModuleSymbolComputer.java(0 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/ModuleSymbol.java(0 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/variable/VariableDescription.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java(17 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BeginTransactionBeforeTryCatchDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommitTransactionOutsideTryCatchDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnostic.java(5 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CommandProvider.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/RenameProvider.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java(6 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java(5 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndexFiller.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MdoRefBuilder.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/UTF8Control.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java(1 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/ModuleSymbolComputer.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/ModuleSymbol.java
🧰 Additional context used
📓 Path-based instructions (3)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndexFiller.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/UTF8Control.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/ClientCapabilitiesHolder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/MeasuresAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/variable/VariableDescription.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BeginTransactionBeforeTryCatchDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/RenameProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CognitiveComplexityComputer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CyclomaticComplexityComputer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CommandProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MdoRefBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommitTransactionOutsideTryCatchDiagnostic.java
**/diagnostics/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BeginTransactionBeforeTryCatchDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommitTransactionOutsideTryCatchDiagnostic.java
build.gradle.kts
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
build.gradle.kts: Use Gradle with Kotlin DSL for build configuration
Be cautious with external dependencies and review security alerts from CodeQL and Dependabot
Files:
build.gradle.kts
🧠 Learnings (11)
📓 Common learnings
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/*.java : Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
📚 Learning: 2025-11-19T09:02:03.154Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3610
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/QueryPackageFoldingRangeSupplier.java:26-26
Timestamp: 2025-11-19T09:02:03.154Z
Learning: В проекте bsl-language-server используется кастомная версия ANTLR4 от 1c-syntax (io.github.1c-syntax:antlr4), которая включает базовый класс org.antlr.v4.runtime.Tokenizer с методами getAst() и getTokens(). SDBLTokenizer и BSLTokenizer наследуются от этого базового класса.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.java
📚 Learning: 2025-04-18T22:46:43.245Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:192-203
Timestamp: 2025-04-18T22:46:43.245Z
Learning: В проекте bsl-language-server класс BSLParser.ExpressionContext наследуется от BSLParserRuleContext, а не напрямую от ParserRuleContext. При работе с ним нужно учитывать специфичные методы BSLParserRuleContext.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java
📚 Learning: 2025-02-10T17:13:41.452Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3408
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ProtectedModuleDiagnostic.java:54-58
Timestamp: 2025-02-10T17:13:41.452Z
Learning: In ProtectedModuleDiagnostic.java, the usage of `Range` field as a diagnostic target is accepted as is, despite being marked as a temporary solution in the comments.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to docs/*/diagnostics/**/*.md : Update diagnostic documentation in both Russian and English with examples of problematic code and fixes
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnostic.java
📚 Learning: 2025-04-18T22:42:30.042Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:88-97
Timestamp: 2025-04-18T22:42:30.042Z
Learning: В реализации ANTLR, если метод `getChildCount()` возвращает значение больше 0, то поле `children` гарантированно не равно null.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java
📚 Learning: 2025-02-10T17:12:56.150Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3408
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ProtectedModuleDiagnostic.java:63-66
Timestamp: 2025-02-10T17:12:56.150Z
Learning: In BSL Language Server, `documentContext.getServerContext().getConfiguration()` is guaranteed to return a non-null value, making null checks unnecessary.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/diagnostics/*.java : Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.java
📚 Learning: 2025-01-20T19:33:59.692Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/codelenses/RunAllTestsCodeLensSupplier.java:55-59
Timestamp: 2025-01-20T19:33:59.692Z
Learning: In Spring, self-injection pattern (using Autowired Lazy on a self-reference field) is required to enable caching through Cacheable when methods are called from within the same class, as Spring's proxy-based AOP doesn't work with direct 'this' references.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
📚 Learning: 2025-01-19T20:47:40.061Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T20:47:40.061Z
Learning: Configuration classes in the BSL Language Server project use mutable collections (HashMap, ArrayList) and Data annotation from Lombok, allowing for modification of configuration properties after initialization.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.java
📚 Learning: 2024-12-27T17:03:11.817Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3364
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java:125-127
Timestamp: 2024-12-27T17:03:11.817Z
Learning: В методе `findAnnotation` класса `AnnotationReferenceFinder` уже проверяется, что у аннотации ровно один параметр, поэтому в `getAnnotationName` не требуется отдельная проверка на пустой список.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: Analyse
- GitHub Check: build
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build
- GitHub Check: Sentry
🔇 Additional comments (45)
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndexFiller.java (1)
250-257: LGTM!Clean refactoring from block lambda to expression lambda. The logic remains unchanged, and the single-expression form is more concise and idiomatic for this use case.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java (1)
65-76: Explicitelsebranch improves readability without changing behaviorThe added
elsewith a clarifying comment makes it explicit that all remaining ternary patterns are intentionally treated as non-errors, while preserving existing behavior of the diagnostic. This is a good readability/refactoring-only change.src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.java (1)
324-331: LGTM! Excellent refactoring to functional style.The refactoring from explicit null-check to
Optional.map/orElseGetimproves readability and follows Java best practices for Optional handling. The logic is preserved while making the code more concise and functional.src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java (6)
95-98: LGTM!Comment reformatting improves readability by wrapping long lines appropriately.
111-114: LGTM!Reformatting the chained method calls and lambda improves readability without changing behavior.
159-161: LGTM!Method signature reformatting with aligned parameters follows good Java style conventions.
172-183: LGTM!Method reformatting improves readability of the fluent API chain.
185-195: LGTM!Consistent reformatting with the sibling overloaded method.
228-229: LGTM!Lambda reformatting improves readability and is consistent with other changes in this file.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CyclomaticComplexityComputer.java (1)
247-249: Explicit else branch clarifies intent.The addition of an explicit else branch with a no-op comment makes it clear that other Tree types are intentionally not processed, rather than accidentally overlooked.
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CognitiveComplexityComputer.java (1)
324-326: Explicit else branch clarifies intent.This change mirrors the adjustment in CyclomaticComplexityComputer.java, maintaining consistency between the two complexity computation implementations. The explicit no-op documents that other Tree types are intentionally not processed.
build.gradle.kts (1)
451-455: LGTM! Task metadata enhances discoverability.Adding
descriptionandgroupproperties to the task improves documentation and makes it easier to discover viagradle tasks.src/main/java/com/github/_1c_syntax/bsl/languageserver/ClientCapabilitiesHolder.java (1)
30-50: Nullable import and field annotation are consistent with Optional-based APIUsing
org.jspecify.annotations.Nullableoncapabilitiestogether withOptional.ofNullable(capabilities)keeps the external contract null-safe while documenting the internal field correctly. Lombok’s@Setterwill align the setter parameter’s nullability with the field type, which matches the intended semantics. As per coding guidelines, continued use of Lombok for boilerplate here is appropriate.Please just confirm this compiles cleanly with your configured JSpecify and Lombok versions in your IDE/CI.
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MdoRefBuilder.java (1)
33-33: LGTM! Proper null-safety annotation added.The
@Nullableannotation on theidentifierparameter correctly documents the existing null-handling logic (lines 105-107). This improves the API contract clarity without changing behavior.Also applies to: 102-102
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java (1)
35-35: LGTM! Proper null-safety annotation added.The
@Nullableannotation on theendTokenparameter correctly documents the existing null-handling logic (line 122). The implementation gracefully falls back tostartTokenwhenendTokenis null.Also applies to: 119-122
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.java (1)
33-33: LGTM! Proper null-safety annotation added.The
@Nullablereturn type annotation correctly documents thatgetVariableNamecan return null (lines 176, 181). The caller at line 97 already handles this case appropriately.Also applies to: 171-171
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.java (1)
27-27: LGTM! Proper null-safety annotations and modern Java syntax.The changes correctly improve null-safety:
- The
@Nullableannotations on thevariableNamefield (line 68) andgetVariableNamereturn type (line 110) document the existing null-handling contract.- The use of
varfor local variable inference (line 114) is appropriate for Java 17 and improves readability.- The caller at line 203 already handles null properly:
if (variableName == null).Also applies to: 68-68, 110-120
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java (2)
90-96: LGTM! Clean refactoring of flag rendering.The extraction of flag collection to a helper method improves modularity. The conditional rendering based on non-empty flags list is correct, and using
String.joinfor comma separation is idiomatic.
98-109: LGTM! Effective use of switch expression.The switch expression appropriately leverages Java 17 features to map return-value-reuse modes to resource keys. The handling of
DONT_USEandUNKNOWNcases with an empty string, followed by conditional rendering, cleanly avoids unnecessary output.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java (1)
32-37: LGTM!Import reorganization is appropriate for focused refactoring work on this file.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnostic.java (1)
37-44: Javadoc formatting changes with<br/>are consistent and safeThe added
<br/>lines uniformly separate the short summary from the detailed description in all affected Javadocs (class and methods), improving readability without changing behavior or tool interaction. No further changes needed here.Also applies to: 90-97, 111-118, 124-131, 143-151
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java (1)
314-314: Good refactoring for consistency.Using
varhere aligns with the existing type inference pattern used throughout this method (lines 298, 299, 305, 309), improving code consistency.src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CommandProvider.java (1)
30-30: LGTM! JSpecify annotation import added.The addition of
org.jspecify.annotations.Nullablealigns with the PR's goal of improving null-safety through standardized annotations.src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/MeasuresAspect.java (1)
37-37: Nullability annotation and cleanup are consistent with existing usageMarking
measureCollectoras@Nullableand adding the corresponding import accurately reflect its actual usage pattern (explicit null checks in all advices and@PreDestroyclearing it via the setter). No functional or style issues detected here; the change improves null-safety metadata while preserving behavior.
Based on learnings, Lombok usage here continues to follow the project’s preference for reducing boilerplate.Also applies to: 53-59
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/RenameProvider.java (1)
37-37: LGTM: Appropriate nullability annotation import.The addition of
org.jspecify.annotations.Nullableimport supports the null-safety annotation added to thegetPrepareRenamemethod, improving API documentation and enabling better static analysis.src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java (2)
70-70:@Nullableimport is appropriate and consistentThe new
org.jspecify.annotations.Nullableimport is necessary for the added annotation and keeps import ordering consistent with the existing style.
167-172: Nullability annotation onthrowablematcheswhenCompletecontractTyping the first parameter as
Voidand markingThrowableas@Nullablecorrectly reflectsCompletableFuture<Void>.whenCompletesemantics and aligns with the explicit null-check inside the handler. No behavior change, only clearer null-safety metadata.src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java (2)
64-64: LGTM: Nullability annotation support added.The import is necessary for the new
@Nullableannotation on the helper method parameter and aligns with the PR's null-safety improvements.
171-171: LGTM: Formatting improvements for readability.Minor formatting adjustments with no functional impact.
Also applies to: 341-343, 563-564
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.java (2)
40-40: LGTM! Nullability annotation correctly documents field lifecycle.The
@Nullableannotation accurately reflects thatapplicationEventPublishercan be null (explicitly set to null indestroy()on line 66), improving null-safety documentation for the codebase.Also applies to: 61-61
104-104: LGTM! Defensive null check improves safety.The added null check guards against NPE and aligns with the
@Nullableannotation on the field. While theoretically redundant ifactiveis properly managed, this defensive check protects against unexpected states.src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java (2)
31-31: Import for nullability annotation is correct and necessaryThe new import cleanly matches the
@Nullableusage onparentbelow; no issues here.
46-48: Nullability ofparentnow explicitly documentedMarking
parentas@Nullableaccurately reflects that root expressions have no parent and will surface this to consumers via Lombok-generated accessors. Please ensure existing callers ofgetParent()either null-check or are updated to respect the new nullability contract.src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/UTF8Control.java (1)
24-24: @nullable annotation correctly documents the return contract.The
@Nullableannotation appropriately indicates that this method can returnnull(as seen on lines 52 and 75). The import and annotation align with the PR's objective of adding nullability metadata across the codebase.The code follows Java 17 conventions with appropriate use of
varfor type inference, and jspecify 1.0.0 is properly configured as a project dependency. Adding@Nullableto this overridden method is fully compatible with the JDK base implementation and poses no compilation concerns.src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.java (1)
38-38: NewLanguageClientimport is correct and usedThe added
org.eclipse.lsp4j.services.LanguageClientimport is required for the explicit lambda parameter below and is not redundant. No issues here.src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java (2)
44-45: Imports forEitherand@Nullablecorrectly match usageThe added imports are consistent with the existing use of
EitheringetAnnotationParamand@Nullablein thecreateParameterssignature; this removes the need for fully qualified names without changing behavior. Please just ensure the module already depends onorg.eclipse.lsp4jandorg.jspecifyin the same versions used elsewhere so there are no mixed@Nullablesources.
240-243:@NullableonparamListaligns with existing null-handlingAnnotating
BSLParser.ParamListContextas nullable matches the current contract (paramListis checked fornulland handled viaCollections.emptyList()), improving static null-safety without altering logic. Verify that thisBSLParser.@Nullable ParamListContextplacement is consistent with how other parser contexts are annotated in the project’s style guide.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java (1)
40-40: LGTM: Import added for nullability annotations.The import is properly placed and necessary for the
@Nullableannotation already used on theprevQueryRangefield.src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java (1)
24-24: Импорт@Nullableвыглядит корректнымДобавление
org.jspecify.annotations.Nullableсогласуется с остальным PR по аннотациям nullability, замечаний к этому импорту нет.src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.java (1)
36-36: Nullability metadata onLanguageClientHolderis consistent and clearUsing
@NullableonlanguageClientHoldertogether with the explicit null check inlogExceptionaccurately reflects the optional dependency and aligns with the jspecify-based null-safety approach. Lombok + setter injection usage also matches the project guidelines.Also applies to: 54-56
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java (3)
29-32: ParserRuleContext import and insideContext signature look consistentImporting
ParserRuleContextand constraininginsideContexttoClass<? extends ParserRuleContext>matches how the helper is used (assignment/return contexts) and tightens the API without changing behavior. I don’t see any issues here; any non-parser-context call sites would now fail at compile time, which is desirable.Also applies to: 709-710
97-103: Consolidated insideStructureOrCorrespondence predicate is correctThe combined boolean expression delegates to the same five specific checks as before and preserves short-circuit behavior. This makes the overall intent clearer without changing the effective logic.
621-634: Conservative use of Optional.orElse(false) for correspondence insert detectionThe new
checkInsideCorrespondenceInsert/checkInsideCorrespondenceInsertFirstParamlogic usesOptional.orElse(false)so that only calls where the variable is confidently inferred as a correspondence (Optional.of(true)) are treated as “inside correspondence”; unknown types (Optional.empty()) now fall back tofalse. This is a reasonable, conservative choice that should reduce false positives, assuming existing tests still cover both structure and correspondence cases. Please ensure the diagnostic tests for magic values aroundВставить/Insertstill pass and cover the ambiguous-type scenario.Also applies to: 644-659, 667-680
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommitTransactionOutsideTryCatchDiagnostic.java (1)
33-58: @nullable annotations on internal state match actual usageMarking
nodeEndTransactionandnodeEndFileas@Nullableand explicitly initializing them tonullaligns with how they are used (checked fornullbefore diagnostics are added and assigned via traversal). This improves null-safety metadata without changing behavior; just make sureorg.jspecify.annotationsis configured consistently across the project/tooling so these annotations are picked up.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BeginTransactionBeforeTryCatchDiagnostic.java (1)
33-58: Consistent @nullable usage for BeginTransaction diagnostic stateApplying
@NullabletonodeBeginTransactionandnodeEndFileand initializing them tonullmatches their lifecycle (set during traversal, checked fornullbefore use). This is consistent with the CommitTransaction diagnostic and improves static nullability information without altering runtime behavior. As with the other file, ensure jspecify annotations are correctly integrated into your build/static analysis setup.
...java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java (1)
147-147: Optional: Added blank lines improve readability.The added blank lines provide visual separation between conditional returns and subsequent logic, which slightly improves readability.
Also applies to: 152-152
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommentedCodeDiagnostic.java (1)
78-84: Inline default initialization ofmethodDescriptionsto simplify constructorThe non-null initialization is a good safeguard. Since the default value is static and independent of constructor parameters, you can move it to the field declaration and drop the explanatory comment to keep the constructor focused on
codeRecognizersetup:- private List<MethodDescription> methodDescriptions; + private List<MethodDescription> methodDescriptions = Collections.emptyList(); @@ public CommentedCodeDiagnostic() { codeRecognizer = new CodeRecognizer(threshold, new BSLFootprint()); - methodDescriptions = Collections.emptyList(); // non-null }This keeps the non-null contract while slightly reducing noise.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.java (1)
55-59: Avoid relying onErrorNodeImplimplementation detailHere you depend on
ErrorNodeImpland itssymbolfield:if (((ErrorNodeImpl) node).symbol.getTokenIndex() == -1) {You can keep the same logic while avoiding the concrete class:
- if (((ErrorNodeImpl) node).symbol.getTokenIndex() == -1) { + if (node.getSymbol().getTokenIndex() == -1) {This makes the code a bit more robust against ANTLR implementation changes and reads clearer.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/capabilities/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/diagnostics/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/documentlink/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/formating/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContext.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommentedCodeDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CachePathProvider.java(3 hunks)
✅ Files skipped from review due to trivial changes (4)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/capabilities/package-info.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContext.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/diagnostics/package-info.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommentedCodeDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/formating/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CachePathProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/documentlink/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/package-info.java
**/diagnostics/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommentedCodeDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java
🧠 Learnings (5)
📚 Learning: 2025-11-19T09:02:03.154Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3610
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/QueryPackageFoldingRangeSupplier.java:26-26
Timestamp: 2025-11-19T09:02:03.154Z
Learning: В проекте bsl-language-server используется кастомная версия ANTLR4 от 1c-syntax (io.github.1c-syntax:antlr4), которая включает базовый класс org.antlr.v4.runtime.Tokenizer с методами getAst() и getTokens(). SDBLTokenizer и BSLTokenizer наследуются от этого базового класса.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.java
📚 Learning: 2025-04-18T22:46:43.245Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:192-203
Timestamp: 2025-04-18T22:46:43.245Z
Learning: В проекте bsl-language-server класс BSLParser.ExpressionContext наследуется от BSLParserRuleContext, а не напрямую от ParserRuleContext. При работе с ним нужно учитывать специфичные методы BSLParserRuleContext.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.java
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/*.java : Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/package-info.java
📚 Learning: 2025-02-10T17:12:56.150Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3408
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ProtectedModuleDiagnostic.java:63-66
Timestamp: 2025-02-10T17:12:56.150Z
Learning: In BSL Language Server, `documentContext.getServerContext().getConfiguration()` is guaranteed to return a non-null value, making null checks unnecessary.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/formating/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/documentlink/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/package-info.java
📚 Learning: 2025-01-19T20:47:40.061Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T20:47:40.061Z
Learning: Configuration classes in the BSL Language Server project use mutable collections (HashMap, ArrayList) and Data annotation from Lombok, allowing for modification of configuration properties after initialization.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CachePathProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/package-info.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Analyse
- GitHub Check: build
- GitHub Check: Analyze the repo with CodeSee
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
🔇 Additional comments (12)
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.java (1)
52-52: LGTM! Good code organization.Moving the constant to the top of the class alongside other constants improves code readability and follows Java best practices for organizing class members.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java (1)
162-162: Excellent refactoring: helper methods correctly made static.Both
hasVariableWithNameandhasClientCompilerDirectivedon't access instance state, so making them static is appropriate and follows Java best practices. The method reference at line 162 is correctly updated to use the static-qualified form.Also applies to: 165-165, 172-172
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CachePathProvider.java (3)
24-24: LGTM!The JSpecify
@Nullableimport is appropriate for null-safety annotations and is required for the parameter annotation added on line 80.
99-99: LGTM!Making
getCurrentDirectory()static is appropriate since it doesn't use any instance state and is purely functional. This is a good refactoring that clarifies the method's independence from instance context.
80-80: LGTM. The@Nullableannotation correctly documents thatfullPathcan be null, which aligns with the null check on line 81 and is validated by test cases (e.g., line 166). This improves null-safety by making the contract explicit. All callers handle the nullable parameter appropriately.src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/formating/package-info.java (1)
25-28: ИмпортNullMarkedразмещён корректно и единообразноАннотация пакета
@NullMarkedи импортorg.jspecify.annotations.NullMarkedоформлены в стандартном для package-info.java виде; поведение не меняется, выглядит хорошо.src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/package-info.java (1)
30-33: Единообразное объявление@NullMarkedдля корневого пакета конфигурацииДобавленный импорт
org.jspecify.annotations.NullMarkedпосле объявления пакета соответствует стилю остальных package-info и делает использование аннотации явным. Замечаний нет.src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/package-info.java (1)
25-28: Корректное оформление аннотации@NullMarkedдля пакета codelensИмпорт
org.jspecify.annotations.NullMarkedпосле объявления пакета оформлен так же, как в остальных configuration-пакетах; это поддерживает единый стиль, поведение не затрагивается.src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/documentlink/package-info.java (1)
25-28: Импорт аннотацииNullMarkedсогласован с остальными пакетамиРазмещение импорта
org.jspecify.annotations.NullMarkedпослеpackageи использование@NullMarkedна пакете выглядят корректно и поддерживают единый стиль package-info в конфигурации.src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.java (2)
27-27: LGTM: Nullability annotation import added.The import for
@Nullablefrom jspecify is correctly added to support the nullability annotation.
33-33: @nullable annotation on entryPoint is appropriate and correct.The annotation accurately documents that
entryPointcan be null in its initial state (not initialized in the constructor). All code that callsgetEntryPoint()handles the nullability appropriately: test code explicitly asserts non-null before use, and production code uses it in null-safe comparisons or guarantees initialization through the builder before any usage. This is a valid documentation improvement that clarifies the nullability contract.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.java (1)
65-71: Stream refactor +varusage look goodUsing
varforast,initialExpectedString,expectedTokens,sj, anderrorTokenkeeps the code concise while the types remain obvious from the initializers. The class-based filtering (ParserRuleContext.class::isInstance/::cast) also makes the stream pipeline cleaner and type-safe without changing behavior. No issues from my side here.Also applies to: 73-75, 80-88
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.java (1)
122-129: Explicit lambda parameter type is fine; consider style consistencyUsing
(LanguageClient languageClient) -> { ... }is type-safe and clear, and matches the new import. For consistency withpublishDiagnostics, which useslanguageClient -> ..., you might optionally align both usages (either both inferred or both explicit), depending on project style preferences.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java (2)
29-32: ParserRuleContext-based helpers are fine; consider explicit null policy forgetExpression(ParserRuleContext)Using
ParserRuleContextdirectly and the genericinsideContexthelper improves reuse and keeps dependencies on ANTLR types clear. One small follow-up:getExpression(ParserRuleContext ctx)currently usesOptional.of(ctx)without a null guard, unlikeinsideContextwhich explicitly handlesexpression == null. If there’s any chance callers might passnull, consider either documenting it as@NotNullor switching to an explicit check/Optional.ofNullableto avoid a possible NPE.Also applies to: 712-719
122-137: Optional/flatMap pipeline for variable type resolution looks correct; boolean encoding could be made more explicitThe refactor around
isSecondParameterOfStructureInsert,getVariableTypeFromCallParamList,getVariableTypeFromCallStatement,getVariableTypeFromAST, and the correspondence checks looks sound:
paramIndex == 1is consistently used for the "second parameter" case.getVariableTypeFromCallParamListcorrectly short-circuits when the call is not anINSERT_METHOD_PATTERNmatch.- The
Optionalchains (flatMaptogetVariableTypeFromCallStatement/getVariableTypeFromAST, thenmap/orElse(false)) preserve the previous empty-check behavior.- The tri-state semantics (
Optional<Boolean>:true= correspondence,false= structure, empty = unknown) are used consistently:
- Structure-only case:
map(typeIsCorrespondence -> !typeIsCorrespondence).orElse(false);- Correspondence-only cases:
variableType.orElse(false);- First-parameter-of-insert just checks
isInsertMethod(...).isPresent().The only downside is readability: the meaning of
truevsfalseon theBooleanis not obvious at call sites. If you touch this again, consider replacingOptional<Boolean>with a small enum (e.g.ContainerType { STRUCTURE, CORRESPONDENCE }) or helper methods likeisStructureType(...)/isCorrespondenceType(...)to make intent self-documenting.Also applies to: 218-225, 254-257, 303-313, 635-637, 661-662, 681-683
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java(5 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java(17 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java
**/diagnostics/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java
🧠 Learnings (6)
📚 Learning: 2025-01-19T20:47:40.061Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T20:47:40.061Z
Learning: Configuration classes in the BSL Language Server project use mutable collections (HashMap, ArrayList) and Data annotation from Lombok, allowing for modification of configuration properties after initialization.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java
📚 Learning: 2025-02-10T17:12:56.150Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3408
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ProtectedModuleDiagnostic.java:63-66
Timestamp: 2025-02-10T17:12:56.150Z
Learning: In BSL Language Server, `documentContext.getServerContext().getConfiguration()` is guaranteed to return a non-null value, making null checks unnecessary.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java
📚 Learning: 2025-01-19T21:34:39.797Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T21:34:39.797Z
Learning: In BSL Language Server configuration classes, immutable collections (Set.of, List.of) should be used for default values, while mutability is achieved through setters generated by Data annotation.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/diagnostics/*.java : Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java
📚 Learning: 2025-04-18T22:46:43.245Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:192-203
Timestamp: 2025-04-18T22:46:43.245Z
Learning: В проекте bsl-language-server класс BSLParser.ExpressionContext наследуется от BSLParserRuleContext, а не напрямую от ParserRuleContext. При работе с ним нужно учитывать специфичные методы BSLParserRuleContext.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java
📚 Learning: 2025-11-19T09:02:03.154Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3610
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/QueryPackageFoldingRangeSupplier.java:26-26
Timestamp: 2025-11-19T09:02:03.154Z
Learning: В проекте bsl-language-server используется кастомная версия ANTLR4 от 1c-syntax (io.github.1c-syntax:antlr4), которая включает базовый класс org.antlr.v4.runtime.Tokenizer с методами getAst() и getTokens(). SDBLTokenizer и BSLTokenizer наследуются от этого базового класса.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build
- GitHub Check: Analyse
🔇 Additional comments (15)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.java (2)
24-39: NewLanguageClientimport is appropriateThe added
org.eclipse.lsp4j.services.LanguageClientimport correctly reflects its new usage in the typed lambda below; no issues here.
64-68: No functional change in field/spacing regionOnly formatting/spacing around
clientSupportsRefreshappears to have changed; there is no behavioral impact to review.src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.java (2)
36-36: Импорт@Nullableиз jspecify выглядит корректнымИмпорт
org.jspecify.annotations.Nullableсогласован с аннотацией поляlanguageClientHolder; в файле нет других источников@Nullable, конфликтов не видно.При необходимости проверьте, что зависимость jspecify действительно подключена в модуле и аннотации обрабатываются в используемом инструменте анализа null‑безопасности.
83-100: РефакторингlogExceptionулучшает null‑безопасность без изменения поведенияКопирование
languageClientHolderв локальную переменную, проверка наnullи ранний выход до отправки сообщения устраняют возможныйNPEпри отсутствии клиента; логика поSentryId.EMPTY_IDи отправке сообщения при наличии клиента сохранена.Имеет смысл прогнать сценарий с отключённым языковым клиентом (или в ранней фазе старта), чтобы убедиться, что исключения по‑прежнему попадают в Sentry, а отсутствие клиента просто тихо подавляет показ сообщения.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java (2)
32-37: Import reordering looks appropriate.The import reorganization follows standard conventions (framework imports followed by Java standard library). However, note that the PR checklist indicates the precommit task hasn't been run yet. Please execute
gradlew precommitbefore finalizing this PR to ensure all formatting and import optimizations are properly applied.As per coding guidelines, imports should be optimized before committing.
120-133: Excellent refactoring with improved control flow.The restructured logic is clearer and more maintainable:
- Early extraction of
expressionwith proper Optional handling- Explicit hierarchical checks with early returns for structure/correspondence and return statements
- Consolidated final condition (
!isNumericExpression(context) || insideCallParam(context)) is more readable than sequential checks- Proper handling of absent expression with explicit
return falseThe refactoring follows Java 17 best practices with appropriate use of
varand makes the decision tree more explicit.src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java (1)
161-185: Javadoc and @nullable now accurately document behavior.The updated
@returnJavadoc and@NullableongetCustomConfigurationRootare consistent with the existing logic (null only when the configured root lies outsidesrcDir). No functional issues here.src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java (4)
70-80: Nullable import is appropriate for new lambda annotation.The added
org.jspecify.annotations.Nullableimport matches the new usage ininitialized()and aligns with the project’s nullability style.
107-137: initialize(): configuration root setup placement looks reasonable.Calling
setConfigurationRoot(params)right after capturing client capabilities keeps initialization concerns localized and does not affect subsequent capability construction.
140-159: Verify downstream handling of possibly-null configurationRoot.
LanguageServerConfiguration.getCustomConfigurationRoot(...)can now returnnull(e.g., when the configured root lies outsiderootPath), and this value is passed directly intocontext.setConfigurationRoot(configurationRoot). This is fine ifServerContext.setConfigurationRootand subsequent use sites are designed to accept and correctly handlenull; otherwise, it could surface as an NPE or unexpected behavior duringpopulateContext().If not already done in this PR, please double-check
ServerContext.setConfigurationRootand its callers to ensure a nullable root is either guarded against or treated as a supported “no configuration root” state.
161-172: whenComplete nullability annotation matches CompletableFuture contract.Annotating the
Throwableparameter as@NullableinwhenComplete((Void unused, @Nullable Throwable throwable) -> { ... })correctly reflects that it can benullon successful completion, and the shutdown/error-logging logic remains unchanged and safe.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java (4)
97-103: CombinedinsideStructureOrCorrespondencecondition is clear and preserves behaviorRefactoring the previous early-return chain into a single short-circuiting boolean expression keeps the evaluation order and semantics while making the control flow easier to scan. No issues here.
263-265: Javadoc alignment/formatting changes are consistent and non-breakingThe updated
@paramalignment and spacing across these methods improves readability and matches typical JavaDoc style without affecting behavior. Nothing to change here.Also applies to: 362-365, 388-391, 417-422, 462-465, 708-711
267-272:CallParamInforecord usage is straightforwardThe
CallParamInforecord cleanly groupscallParam,paramIndex, andcallParamList, and its usage ingetCallParamInfois idiomatic. The empty body is fine and leaves room for future helper methods if needed.
484-495: Structure-constructor detection refactor is correct and more idiomaticThe refactored
checkInsideStructureConstructorandisValidStructureConstructorParameterkeep the previous behavior while simplifying the flow:
findCallParamContextstill guards the path.getStructureTypeName+map(...).orElse(false)is equivalent to the oldisEmpty()/get()pattern.- Parameter index handling remains: index
0excluded, others considered, withFixedStructureshort-circuiting totrue.- For non-fixed structures, validating the first parameter as a STRING token for field names is unchanged.
Overall this Optional-based version is cleaner without introducing new edge cases.
Also applies to: 550-575
theshadowco
force-pushed
the
feature/SQFixes251111
branch
from
c422ff5 to
a0b8fbb
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java (1)
80-99: Обработкаnullвputтеперь соответствует контракту, но комментарий вgetвводит в заблуждение
- В
put(Object key, @Nullable Object value)теперь явно выбрасываетсяNullPointerExceptionприvalue == null, что согласуется сallowNullValues = falseи документом класса — это хороший и ожидаемый фикс.- В
get(Object key, Callable<T> valueLoader)условие на Line 86:if (value != null) { // если нет в кеше, загружаем из valueLoader return (T) value; }Комментарий описывает поведение ветки
else, а неif.Я бы предложил поправить комментарий, чтобы он соответствовал коду, например:
- if (value != null) { // если нет в кеше, загружаем из valueLoader - return (T) value; - } + if (value != null) { // значение есть в кеше, просто возвращаем его + return (T) value; + } + // если значения нет в кеше, загружаем из valueLoaderТак будет явно видно, какая ветка за что отвечает.
Also applies to: 101-112
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java (1)
134-148: Move@Nullablefrom method declaration to return typeThe
@Nullableannotation from JSpecify is a type-use only annotation (@Target(TYPE_USE)). When placed on the method declaration as shown:@Nullable private MessageActionItem waitForPermission(CompletableFuture<MessageActionItem> sendQuestion) {It is syntactically incorrect and will not mark the return type as nullable. The annotation must be placed on the type itself:
private @Nullable MessageActionItem waitForPermission( CompletableFuture<MessageActionItem> sendQuestion ) {This aligns with the project's Style Guide, which specifies using
@org.jspecify.annotations.Nullableto indicate nullable return values.
🧹 Nitpick comments (10)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java (1)
74-76: Consider removing the unnecessary else branch.Empty else branches are typically omitted in Java as they serve no functional purpose. The comment clarifies intent but could be placed above the entire if-else chain if needed.
Apply this diff to remove the no-op else branch:
} else if (trueBranch != SKIPPED_RULE_INDEX || falseBranch != SKIPPED_RULE_INDEX) { diagnosticStorage.addDiagnostic(ctx); - } else { - // остальное - не ошибка }src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommitTransactionOutsideTryCatchDiagnostic.java (2)
52-53: Consider using consistent annotation style.Both fields are annotated as nullable but use different styles:
- Line 52:
@Nullable ParserRuleContext(declaration annotation)- Line 53:
BSLParser.@Nullable StatementContext(type-use annotation)For consistency, consider using the type-use annotation style for both fields.
Apply this diff for consistency:
- private @Nullable ParserRuleContext nodeEndTransaction; + private @Nullable ParserRuleContext nodeEndTransaction; private BSLParser.@Nullable StatementContext nodeEndFile;Or alternatively, use declaration annotations for both (though type-use is more precise with qualified types):
- private @Nullable ParserRuleContext nodeEndTransaction; - private BSLParser.@Nullable StatementContext nodeEndFile; + private @Nullable ParserRuleContext nodeEndTransaction; + private @Nullable BSLParser.StatementContext nodeEndFile;
55-58: Optional: Redundant explicit null initialization.Object fields in Java are automatically initialized to
null, making the explicit assignments redundant. You may remove the constructor entirely or, per the coding guidelines recommending Lombok, consider using@NoArgsConstructorif you need an explicit constructor.Option 1: Remove the constructor entirely (fields default to null):
- public CommitTransactionOutsideTryCatchDiagnostic() { - nodeEndTransaction = null; - nodeEndFile = null; - } -Option 2: Use Lombok (if used elsewhere in the codebase):
+@NoArgsConstructor public class CommitTransactionOutsideTryCatchDiagnostic extends AbstractVisitorDiagnostic { // ... - public CommitTransactionOutsideTryCatchDiagnostic() { - nodeEndTransaction = null; - nodeEndFile = null; - }Based on learnings, Lombok annotations help reduce boilerplate code.
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java (2)
91-113: Refactor of flags and reuse block looks correct; clarify intent for omitted reuse statesThe extraction of flags and the new
reuseKeyswitch look sound and keep the builder logic readable. One thing to double‑check: forDONT_USEandUNKNOWNyou now intentionally skip rendering any “return values reuse” line (empty key). If previous behavior showed something for those states, this might be a behavior change.If the silent omission is desired, the implementation is fine. If not, consider mapping them to explicit resource keys instead of
"". Also, instead of using an empty string sentinel, you could useOptional<String>or a nullableStringfor slightly clearer intent, e.g.:String reuseKey = switch (returnValueReuse) { case DURING_REQUEST -> "duringRequest"; case DURING_SESSION -> "duringSession"; case DONT_USE, UNKNOWN -> null; }; if (reuseKey != null) { var reuseHeader = "**" + getResourceString("returnValuesReuse") + ":** "; moduleInfoBuilder.add(reuseHeader + getResourceString(reuseKey)); }
115-140: NewgetModuleFlagshelper is straightforward and improves readabilityThe extracted helper mirrors the previous inline logic, keeps responsibilities separated, and returns a
List<String>which is convenient for callers. No functional issues spotted.If you want a tiny cleanup, you could use
var flags = new ArrayList<String>();consistently withvarelsewhere, but that’s purely stylistic and optional.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BeginTransactionBeforeTryCatchDiagnostic.java (1)
52-58: Nullability on state fields matches their lifecycle; minor style tweak possibleAnnotating
nodeBeginTransactionandnodeEndFileas nullable aligns with their initialization tonulland the explicit null checks before use, which should help static analysis without changing behavior. If there is no project-wide preference, you might consider simplifyingprivate BSLParser.@Nullable StatementContext nodeEndFile;toprivate @Nullable BSLParser.StatementContext nodeEndFile;for slightly clearer readability and consistency withnodeBeginTransaction.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.java (1)
78-88: Consider skipping the dimension-table diagnostic when the cube is missing to avoid cascaded errorsThe added check for missing cube dimension tables is logically sound and null-safe; the
cubeDimTable != nullguard and prior filter chain prevent any issues.However, the current logic reports both a missing cube and a missing dimension table when only the cube is absent. When
nonCubeExists(eds)is true,getCubeDimTable(eds)will also be empty (because the cube lookup fails upstream), triggering both diagnostics for a single root cause. To improve the user experience, consider skipping the dimension-table check when the cube is missing:- .forEach((SDBLParser.ExternalDataSourceTableContext eds) -> { - if (nonCubeExists(eds)) { - diagnosticStorage.addDiagnostic(eds.cubeName, info.getMessage(eds.cubeName.getText())); - } - - var cubeDimTable = eds.tableName; - - if (cubeDimTable != null && nonCubeDimTableExists(eds)) { - diagnosticStorage.addDiagnostic(cubeDimTable, info.getMessage(eds.tableName.getText())); - } - }); + .forEach((SDBLParser.ExternalDataSourceTableContext eds) -> { + var cubeMissing = nonCubeExists(eds); + if (cubeMissing) { + diagnosticStorage.addDiagnostic(eds.cubeName, info.getMessage(eds.cubeName.getText())); + } + + var cubeDimTable = eds.tableName; + + if (!cubeMissing && cubeDimTable != null && nonCubeDimTableExists(eds)) { + diagnosticStorage.addDiagnostic(cubeDimTable, info.getMessage(eds.tableName.getText())); + } + });The test file
QueryToMissingMetadataDiagnosticTest.javaalready exists and covers both missing cubes and missing dimension tables, including the cascaded scenario on line 80 where both errors are expected. Update the test expectations if you adopt the refactor above.src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CachePathProvider.java (1)
24-25: Сделайте nullabilityfullPathконсистентной во всех перегрузкахgetCachePathСейчас 3‑аргументная версия помечает
fullPathкак@Nullableи корректно обрабатываетnull:public Path getCachePath(String basePath, @Nullable String fullPath, int instanceNumber) { if (fullPath != null && !fullPath.isEmpty()) { return Path.of(fullPath); } ... }Но 2‑аргументная перегрузка по сигнатуре принимает ненулевой
String fullPath:public Path getCachePath(String basePath, String fullPath) { return getCachePath(basePath, fullPath, 0); }Если по договору вы действительно допускаете
null(а код 3‑аргументного метода к этому готов), имеет смысл:
- Пометить параметр
fullPathв 2‑аргументной версии как@Nullable.- Обновить javadoc, явно указав, что
nullи пустая строка означают "использовать путь по умолчанию".Например:
- public Path getCachePath(String basePath, String fullPath) { + public Path getCachePath(String basePath, @Nullable String fullPath) { return getCachePath(basePath, fullPath, 0); }Так nullability станет однозначной для статического анализа и читателя кода.
Also applies to: 57-59, 80-92, 99-105
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java (2)
267-272: Remove unnecessary empty constructor body.Records automatically provide a canonical constructor, so the explicit empty body adds no value and should be removed.
Apply this diff:
private record CallParamInfo( BSLParser.CallParamContext callParam, int paramIndex, BSLParser.CallParamListContext callParamList - ) { - } + ) { }Or simply:
private record CallParamInfo( BSLParser.CallParamContext callParam, int paramIndex, BSLParser.CallParamListContext callParamList - ) { - } + ) { }
682-682: Simplify nested Optional pattern.The
filter + isPresentpattern can be simplified usingflatMapfor better readability and idiomatic Optional usage.Apply this diff:
- return doCallOpt.filter(doCallContext -> isInsertMethod(doCallContext).isPresent()).isPresent(); + return doCallOpt.flatMap(AbstractMagicValueDiagnostic::isInsertMethod).isPresent();
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (53)
build.gradle.kts(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java(5 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/ClientCapabilitiesHolder.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/capabilities/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/diagnostics/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/documentlink/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/formating/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContext.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CognitiveComplexityComputer.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CyclomaticComplexityComputer.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/ModuleSymbolComputer.java(0 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/ModuleSymbol.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/variable/VariableDescription.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java(17 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BeginTransactionBeforeTryCatchDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommentedCodeDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommitTransactionOutsideTryCatchDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnostic.java(5 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CachePathProvider.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CommandProvider.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/RenameProvider.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java(6 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java(5 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndexFiller.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MdoRefBuilder.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/UTF8Control.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/ModuleSymbolComputer.java
✅ Files skipped from review due to trivial changes (3)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ServerCallsInFormEventsDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/package-info.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/formating/package-info.java
🚧 Files skipped from review as they are similar to previous changes (25)
- build.gradle.kts
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommentedCodeDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/variable/VariableDescription.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CognitiveComplexityComputer.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ParseErrorDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContext.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/RefOveruseDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/DescriptionFormatter.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/DiagnosticProvider.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/package-info.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CacheConfiguration.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnsafeFindByCodeDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/capabilities/package-info.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/documentlink/package-info.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/diagnostics/package-info.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MdoRefBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/ClientCapabilitiesHolder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/references/AnnotationReferenceFinder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Ranges.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CommandProvider.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/RenameProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CachePathProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/ModuleSymbol.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommitTransactionOutsideTryCatchDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndexFiller.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CyclomaticComplexityComputer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/UTF8Control.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BeginTransactionBeforeTryCatchDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java
**/diagnostics/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommitTransactionOutsideTryCatchDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BeginTransactionBeforeTryCatchDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java
🧠 Learnings (10)
📓 Common learnings
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/*.java : Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/diagnostics/*.java : Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryToMissingMetadataDiagnostic.java
📚 Learning: 2025-11-19T09:02:03.154Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3610
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/QueryPackageFoldingRangeSupplier.java:26-26
Timestamp: 2025-11-19T09:02:03.154Z
Learning: В проекте bsl-language-server используется кастомная версия ANTLR4 от 1c-syntax (io.github.1c-syntax:antlr4), которая включает базовый класс org.antlr.v4.runtime.Tokenizer с методами getAst() и getTokens(). SDBLTokenizer и BSLTokenizer наследуются от этого базового класса.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.java
📚 Learning: 2025-04-18T22:46:43.245Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:192-203
Timestamp: 2025-04-18T22:46:43.245Z
Learning: В проекте bsl-language-server класс BSLParser.ExpressionContext наследуется от BSLParserRuleContext, а не напрямую от ParserRuleContext. При работе с ним нужно учитывать специфичные методы BSLParserRuleContext.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java
📚 Learning: 2025-01-19T20:47:40.061Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T20:47:40.061Z
Learning: Configuration classes in the BSL Language Server project use mutable collections (HashMap, ArrayList) and Data annotation from Lombok, allowing for modification of configuration properties after initialization.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/CachePathProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.java
📚 Learning: 2025-02-10T17:12:56.150Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3408
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ProtectedModuleDiagnostic.java:63-66
Timestamp: 2025-02-10T17:12:56.150Z
Learning: In BSL Language Server, `documentContext.getServerContext().getConfiguration()` is guaranteed to return a non-null value, making null checks unnecessary.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/ModuleSymbol.java
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/diagnostics/*Test.java : Write comprehensive unit tests for each diagnostic including test cases for edge cases, following existing test patterns
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java
📚 Learning: 2025-01-20T19:33:59.692Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/codelenses/RunAllTestsCodeLensSupplier.java:55-59
Timestamp: 2025-01-20T19:33:59.692Z
Learning: In Spring, self-injection pattern (using Autowired Lazy on a self-reference field) is required to enable caching through Cacheable when methods are called from within the same class, as Spring's proxy-based AOP doesn't work with direct 'this' references.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
📚 Learning: 2025-04-18T22:42:30.042Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:88-97
Timestamp: 2025-04-18T22:42:30.042Z
Learning: В реализации ANTLR, если метод `getChildCount()` возвращает значение больше 0, то поле `children` гарантированно не равно null.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java
📚 Learning: 2025-02-10T17:13:41.452Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3408
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ProtectedModuleDiagnostic.java:54-58
Timestamp: 2025-02-10T17:13:41.452Z
Learning: In ProtectedModuleDiagnostic.java, the usage of `Range` field as a diagnostic target is accepted as is, despite being marked as a temporary solution in the comments.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build
- GitHub Check: Benchmark
- GitHub Check: Analyse
🔇 Additional comments (45)
src/main/java/com/github/_1c_syntax/bsl/languageserver/references/ReferenceIndexFiller.java (1)
247-257: Formatting-only refactor ofifPresentlooks goodThe
findCommonModule(...).ifPresent(...)block is now just reformatted; control flow, null handling, and arguments toaddModuleReferenceare unchanged. This keeps behavior intact while matching the surrounding style.src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/CyclomaticComplexityComputer.java (1)
247-249: Explicit no-op else keeps behavior unchanged and clarifies intentThe added
else { // no-op }keeps the previous behavior of ignoring non‑MemberContext/OperationContextchildren while making that choice explicit (and likely silencing static analysis warnings). Looks good as-is; no functional or style issues from the perspective of the current guidelines.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UselessTernaryOperatorDiagnostic.java (1)
113-113: Good refactoring to make the method static.The method doesn't access any instance state, so making it static is appropriate and follows Java best practices.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTemporaryFileDeletionDiagnostic.java (2)
33-33: LGTM! Appropriate import for nullability annotation.The JSpecify
@Nullableimport is correctly added to support the nullability annotation on thegetVariableNamemethod.
171-171: LGTM! Nullability annotation correctly applied.The
@Nullableannotation accurately documents thatgetVariableNamecan returnnull(lines 176, 181). The existing null check at line 98 confirms proper handling by callers.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java (3)
43-43: LGTM: Redundant initialization removed.The explicit initialization to
falseis unnecessary since boolean fields default tofalsein Java. This simplification is valid and aligns with the PR's refactoring objectives.
60-60: LGTM: Formatting corrections applied.Added spaces after
ifkeywords, conforming to standard Java style conventions.Also applies to: 74-74
68-73: LGTM: Comment formatting improved.The multi-line block format enhances readability for this explanatory comment without changing its content.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CommitTransactionOutsideTryCatchDiagnostic.java (1)
33-33: LGTM: Null-safety annotation import.Adding the JSpecify
@Nullableannotation improves null-safety across the codebase.src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.java (1)
27-33: NullableentryPointlooks correct; verify Lombok + JSpecify interplayMarking
entryPointas@Nullablealigns with the broader null‑safety refactor and keeps the Lombok-based API minimal. Please just verify that your Lombok configuration propagates the field’s@Nullableto the generatedgetEntryPoint()return type in a way that your JSpecify-aware tools understand; if not, you may need to add method-level/nullness annotations via Lombok’sonMethod_or similar configuration. Based on learnings, this continues the preferred Lombok-based style while tightening null contracts.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TransferringParametersBetweenClientAndServerDiagnostic.java (3)
143-155: Нет функциональных изменений вisCachedValueParameterИзменения в этом блоке носят только форматный характер (пустые строки), логика кэш‑параметров осталась прежней и выглядит корректной.
157-163: Корректный переход на статический method referenceЗамена
.anyMatch(this::hasClientCompilerDirective)на.anyMatch(TransferringParametersBetweenClientAndServerDiagnostic::hasClientCompilerDirective)соответствует переводу хелпера вstaticи убирает лишнюю привязку к экземпляру. Поведения не меняет.
165-179: Статические хелперы оформлены корректноПеревод
hasVariableWithNameиhasClientCompilerDirectiveвprivate staticоправдан (они не используют состояние объекта), сигнатуры и логика не изменились. ИспользованиеOptional::streamи сравнение директив выглядит корректным для целевой Java 17.src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java (1)
34-34: Import ofjava.util.Listis appropriateThe added
Listimport matches the new helper’s return type and keeps the public contract on the abstract type rather thanArrayList, which is good API style.src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/ModuleSymbol.java (2)
26-28: LGTM! Import optimization for Lombok annotations.The addition of nested annotation imports (
Builder.DefaultandEqualsAndHashCode.Include) enables cleaner shorthand notation throughout the file, which is a standard Lombok practice for reducing verbosity.
55-66: LGTM! Consistent use of shorthand Lombok annotations.The conversion from fully qualified annotation names (
@EqualsAndHashCode.Include,@Builder.Default) to their shorthand equivalents (@Include,@Default) is correct and improves code readability. The usage is appropriate:
@Includemarks fields for inclusion in equals/hashCode (consistent with@EqualsAndHashCode(onlyExplicitlyIncluded = true))@Defaultprovides a default value for the buildersrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/UTF8Control.java (3)
24-25: LGTM: Import addition aligns with project-wide nullability annotations.The import of
@Nullablefrom org.jspecify is appropriate and consistent with the project's broader refactoring to add nullability annotations throughout the codebase.
50-51: LGTM: Type inference with var reduces verbosity appropriately.The use of
varfor local variables is appropriate here as the types are clear from the method calls on the right-hand side (toBundleName(),toResourceName(),getResource(),openConnection(), andnew InputStreamReader()). This modernizes the code while maintaining readability.Also applies to: 55-55, 57-57, 68-68
44-48: @nullable annotation correctly documents null return possibility.The
@Nullableannotation on the return type is appropriate since the method can returnnull(line 75 returnsbundlewhich is initialized tonullat line 52 and remains null when the resource stream is not found). This makes the contract explicit for callers. The annotation usesorg.jspecify.annotations.Nullable, which is a modern, tool-agnostic approach suitable for Java 17.src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.java (3)
36-36: LGTM!The import addition is properly placed and supports the nullability annotation added to the
languageClientHolderfield.
54-54: LGTM!The
@Nullableannotation appropriately markslanguageClientHolderas an optional dependency, making the contract explicit and enabling better null-safety checks.
83-100: Excellent refactoring for thread safety and null handling.The changes properly address the nullable
languageClientHolderfield while improving thread safety:
TOCTOU prevention: Copying
languageClientHolderto the localclientHoldervariable (line 89) prevents race conditions in the async context where the field could theoretically be set to null between the null check and usage.Defensive programming: Early returns (lines 86-88, 90-92) clearly handle invalid states before performing unnecessary work.
Null safety: Explicit null check on
clientHolderprevents potential NPE when the dependency is not injected.The refactoring maintains the same functional behavior while making the code more robust in concurrent scenarios.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BeginTransactionBeforeTryCatchDiagnostic.java (1)
33-33: Nullable import is correctly introduced and usedThe
org.jspecify.annotations.Nullableimport is justified by the field annotations below and is not unused; no issues here.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicNumberDiagnostic.java (3)
32-37: Import reordering is consistent with styleImports are now grouped logically (project, third‑party, then JDK) with clear separation; no functional impact and aligns with typical Java import organization.
99-99: Whitespace-only changeThis line change is purely formatting with no behavioral effect; safe to keep as is.
120-133: RefactoredisWrongExpressionlogic looks correct; verify diagnostics coverageThe new Optional-based flow in
isWrongExpressionis clearer: it preserves the early return for index access, explicitly skips structure/correspondence, flags return statements, and otherwise applies the existing numeric/call-param checks. I don’t see correctness or null-safety issues here.Given this affects when the magic-number diagnostic is raised, please ensure existing tests and documentation for this diagnostic still match the updated conditions (or add/update them if needed), along with the resource bundle messages, as required for diagnostics. As per coding guidelines and retrieved learnings for diagnostics.
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java (1)
31-32: Аннотация@Nullableдляparentкорректно отражает семантикуКорневые узлы дерева действительно могут не иметь родителя, так что пометка
parentкак@Nullableи добавленный импорт jspecify выглядят уместно и консистентно с остальным PR.Also applies to: 46-48
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java (1)
33-34: Перевод на jspecify@Nullableиvarв рекурсивном поиске выглядит безопаснымИмпорт
org.jspecify.annotations.Nullableсогласуется с уже использующими@Nullableметодами, а замена явногоOptional<TerminalNode>наvarвfindTerminalNodeContainsPositionне меняет тип и поведение, улучшая читаемость.Also applies to: 292-321
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.java (1)
40-41: Защита отnullдляapplicationEventPublisherулучшает устойчивость аспектаАннотация
@Nullableдля поля и дополнительная проверкаapplicationEventPublisher == nullвpublishEventлогично дополняют флагactiveи убирают потенциальныйNullPointerExceptionво время остановки/инициализации бинов, при этом внешний контракт аспекта не меняется.Also applies to: 60-62, 103-109
src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java (1)
70-71: Использованиеvarи явной@Nullableдляthrowableделает код инициализации чище
- Перевод локальных переменных (
capabilities,rootUri,configurationRoot, фабрика/экзекутор) наvarздесь прозрачен по типам и улучшает читаемость без изменения логики.- Сигнатура
whenComplete((Void unused, @Nullable Throwable throwable) -> ...)точно отражает контрактCompletableFuture.runAsync(результатVoid,throwableможет бытьnull), при этом обработка ошибок (LOGGER.error(...)) осталась прежней.По совокупности изменений поведение
initialize/initializedне меняется, только становится более явной nullability.Also applies to: 109-113, 146-159, 161-173
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/RenameProvider.java (1)
37-38: @nullable Range annotation and null-handling are correctly implementedThe transition to
@Nullable RangeingetPrepareRenameis well-designed. The method correctly returnsnullwhen no suitableSourceDefinedSymbolis found, which aligns properly with jspecify annotations:public @Nullable Range getPrepareRename(DocumentContext documentContext, TextDocumentPositionParams params) { return referenceResolver.findReference(documentContext.getUri(), params.getPosition()) .filter(Reference::isSourceDefinedSymbolReference) .map(Reference::getSelectionRange) .orElse(null); }Null handling is correctly implemented at all call sites:
- Test cases explicitly validate both null and non-null scenarios (
assertThat(range).isNull()andassertThat(range).isEqualTo(...))- Production code in
BSLTextDocumentServicewraps the result inEither3.forFirst(), which is the standard LSP4J pattern for handling optional return valuessrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java (13)
29-32: LGTM!Import organization is clean and follows standard conventions.
97-103: LGTM!Replacing sequential early-returns with a single return expression improves readability and maintainability.
128-136: LGTM!The refactoring to use functional-style Optional handling (map/orElse) is more idiomatic and eliminates explicit emptiness checks.
224-224: LGTM!Using
flatMapfor Optional chaining is the idiomatic functional approach.
256-256: LGTM!Using
flatMapfor Optional chaining is the idiomatic functional approach.
310-312: LGTM!Using
flatMapfor Optional chaining is the idiomatic functional approach.
362-365: LGTM!Javadoc parameter alignment improves documentation consistency.
Also applies to: 387-390, 417-421, 461-463, 708-710
490-494: LGTM!The functional-style Optional handling with map/orElse is cleaner and more maintainable.
559-568: LGTM!Additional blank lines improve readability by visually separating logical sections.
636-636: LGTM!The
orElse(false)pattern is appropriate for this boolean check.
661-661: LGTM!The
orElse(false)pattern is appropriate for this boolean check.
712-713: LGTM!Parameter alignment in the method signature improves consistency.
38-38: The code at this location is clean with no debug artifacts, commented-out code, or style issues. AbstractMagicValueDiagnostic is properly tested through its concrete implementations (MagicDateDiagnostic and MagicNumberDiagnostic), which have comprehensive test suites. No action required on this file.src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java (1)
72-79: Explicitly nullableexecutereturn type matches Sentry’s contractMaking
executereturn@Nullable SentryEventcorrectly documents that returningnulldrops the event, which aligns withSentryOptions.BeforeSendCallbacksemantics. The implementation already behaved this way (return nullwhensendToSentry()is false), so this change improves API clarity without altering behavior. (javadoc.io)One thing to be aware of: dropping Lombok’s
@NonNulloneventandhintalso removes Lombok-generated runtime null checks. If you still want hard guarantees here (beyond Sentry’s own@NotNullcontract), consider adding JSpecify@NonNulltype‑use annotations later as part of the broader nullness rollout.Please re-run your usual build (e.g.,
./gradlew compileJavaorgradlew precommit) to confirm there are no signature/override mismatches against the Sentry dependency version in this project.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java (1)
69-73: Potential null safety issue:buildExpressionTreereturns non-nullable but calls nullable getter.
buildExpressionTreedeclares a non-nullableBslExpressionreturn type but delegates togetExpressionTree()which is now@Nullable. Callers may receive null unexpectedly.Consider either:
- Adding
@NullabletobuildExpressionTreereturn type for consistency- Adding a null check with fallback (e.g., return
ErrorExpressionNode)- public static BslExpression buildExpressionTree(BSLParser.ExpressionContext ctx) { + public static @Nullable BslExpression buildExpressionTree(BSLParser.ExpressionContext ctx) { var instance = new ExpressionTreeBuildingVisitor(); instance.visitExpression(ctx); return instance.getExpressionTree(); }src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java (2)
393-413: Поиск присваивания берёт первое совпадение, а не ближайшее предшествующее (возможная неверная инференция типа).
findAssignmentInCodeBlockсейчас возвращает первое присваивание переменной в блоке, которое находится текстуально доbeforeNode. Если переменная несколько раз переопределяется до вызова (x = New Structure(); x = New Map(); x.Insert(...);), метод вернёт самое раннее присваивание, а не последнее перед вызовом, и тип может быть определён неверно.Имеет смысл искать последнее присваивание до позиции:
- for (var statement : codeBlock.statement()) { - if (statement == null) { - continue; - } - - var assignmentOpt = findAssignmentInStatement(statement, beforeLine, beforeChar, variableName); - if (assignmentOpt.isPresent()) { - return assignmentOpt; - } - } - - return Optional.empty(); + var statements = codeBlock.statement(); + for (var i = statements.size() - 1; i >= 0; i--) { + var statement = statements.get(i); + if (statement == null) { + continue; + } + + var assignmentOpt = findAssignmentInStatement(statement, beforeLine, beforeChar, variableName); + if (assignmentOpt.isPresent()) { + return assignmentOpt; + } + } + + return Optional.empty();Такое изменение сделает поведение ближе к описанию в JavaDoc (“до указанного узла”) и фактическому порядку выполнения кода.
692-719:insideContext()should traverse parent chain like other similar methods in this class.The current implementation only checks the immediate parent via
contextClass.isInstance(expression.getParent()). However, expressions can be nested within intermediate nodes (parentheses, unary operators, etc.), causing false negatives. Other methods in the same file—findCallStatement()(line 162),findCallParamContext()(line 506)—already traverse the ancestor chain with depth limits. TheinsideContext()method should follow the same pattern:- protected static boolean insideContext(BSLParser.@Nullable ExpressionContext expression, - Class<? extends ParserRuleContext> contextClass) { - if (expression == null) { - return false; - } - return contextClass.isInstance(expression.getParent()); - } + protected static boolean insideContext(BSLParser.@Nullable ExpressionContext expression, + Class<? extends ParserRuleContext> contextClass) { + if (expression == null) { + return false; + } + + ParserRuleContext current = expression.getParent(); + for (int i = 0; i < MAX_PARENT_TRAVERSAL_DEPTH_FOR_CALL_PARAM && current != null; i++) { + if (contextClass.isInstance(current)) { + return true; + } + current = current.getParent(); + } + + return false; + }Since
insideSimpleAssignment()andinsideReturnStatement()are used in critical diagnostic decision logic (MagicDateDiagnostic,MagicNumberDiagnostic), verify existing test suites pass with this change.
🧹 Nitpick comments (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphWalker.java (1)
27-38: Good null-safety improvement; consider adding a diagnostic messageUsing
Objects.requireNonNullongraph.getEntryPoint()is a solid way to enforce the invariant thatcurrentNodeis never null afterstart(). To make failures easier to diagnose, consider adding a message:currentNode = Objects.requireNonNull( graph.getEntryPoint(), "ControlFlowGraph entry point must not be null when starting walker" );This keeps the behavior (fail fast) while providing clearer context if the invariant is violated.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java (1)
90-102:Objects.requireNonNullusage is correct; consider a tiny cleanupThe explicit
Objects.requireNonNull(node.getParent())in bothaddDiagnosticoverloads accurately encodes the non‑null precondition and matches the checks invisitBinaryOperation/visitUnaryOperation. Functionally this is safe and improves null-safety signaling.If you want to tighten it a bit, you could:
- Reuse the already validated parent instead of re-fetching it, and/or
- Extract the duplicated
startTokencomputation into a small private helper takingBslExpression parent.For example:
- private void addDiagnostic(BinaryOperationNode node) { - var startToken = Trees.getTokens(Objects.requireNonNull(node.getParent()).getRepresentingAst()) + private void addDiagnostic(BinaryOperationNode node) { + var parent = Objects.requireNonNull(node.getParent()); + var startToken = Trees.getTokens(parent.getRepresentingAst()) .stream() .findFirst() .orElseThrow();Same pattern can be applied to the unary case or factored into a common helper. Optional, as current code is already correct.
Also applies to: 104-116
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DataExchangeLoadingDiagnostic.java (1)
119-122: @nullable return is correct; consider explicit Optional usage around sub-contextMarking
getSubContextas@Nullable ParserRuleContextcorrectly reflects thatTrees.getAncestorByRuleIndexmay returnnull, without changing behavior. To make the null contract clearer and avoid potential NPEs or nullability warnings aroundsymbolTree.getMethodSymbol(getSubContext(ctx)), you could optionally wrap the sub-context inOptional.ofNullable:- private Optional<MethodSymbol> methodSymbol(BSLParser.ProcDeclarationContext ctx) { - return Optional.of(documentContext.getSymbolTree()) - .flatMap(symbolTree -> symbolTree.getMethodSymbol(getSubContext(ctx))); - } + private Optional<MethodSymbol> methodSymbol(BSLParser.ProcDeclarationContext ctx) { + return Optional.of(documentContext.getSymbolTree()) + .flatMap(symbolTree -> + Optional.ofNullable(getSubContext(ctx)) + .flatMap(symbolTree::getMethodSymbol) + ); + }This is not strictly required if grammar guarantees a non-null ancestor, but it makes the nullability intent explicit and keeps static analysis happy.
If you adopt this pattern, please verify that the signature of
getMethodSymbolis compatible with a potentially absent sub-context and that diagnostics behavior (presence/absence of findings) remains unchanged in your existing tests.Also applies to: 132-134
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (47)
build.gradle.kts(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/LanguageClientHolder.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/MeasuresAspect.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/CfgBuildingParseTreeVisitor.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphWalker.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/cli/lsp/FileAwarePrintWriter.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/codelenses/AbstractRunTestsCodeLensSupplier.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/diagnostics/DiagnosticsOptions.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContext.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/DiagnosticIgnoranceComputer.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/MethodSymbol.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/DescriptionReader.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/variable/VariableDescription.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.java(19 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CreateQueryInCycleDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DataExchangeLoadingDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DuplicatedInsertionIntoCollectionDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FieldsFromJoinsWithoutIsNullDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseLikeInQueryDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseOfStrTemplateDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IsInRoleMethodDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicDateDiagnostic.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTempStorageDeletionDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/NestedFunctionInParametersDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/SelectTopWithoutOrderByDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TimeoutsInExternalResourcesDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedParametersDiagnostic.java(0 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsageWriteLogEventDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingExternalCodeToolsDiagnostic.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingThisFormDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/metadata/DiagnosticInfo.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/CodeBlockFoldingRangeSupplier.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/LanguageClientAwareAppender.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/jsonrpc/DiagnosticParams.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SelectionRangeProvider.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/Symbol.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/SymbolOccurrence.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/DiagnosticHelper.java(4 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.java(4 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/UTF8Control.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ErrorExpressionNode.java(2 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java(5 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UnusedParametersDiagnostic.java
✅ Files skipped from review due to trivial changes (21)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/jsonrpc/DiagnosticParams.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SelectionRangeProvider.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/diagnostics/DiagnosticsOptions.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/description/DescriptionReader.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/MethodSymbol.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/LanguageClientHolder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/SymbolOccurrence.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/CfgBuildingParseTreeVisitor.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/cli/lsp/FileAwarePrintWriter.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/SelectTopWithoutOrderByDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/NestedFunctionInParametersDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/codelenses/AbstractRunTestsCodeLensSupplier.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/CreateQueryInCycleDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsageWriteLogEventDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/CodeBlockFoldingRangeSupplier.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/TimeoutsInExternalResourcesDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IsInRoleMethodDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/references/model/Symbol.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MissingTempStorageDeletionDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/DiagnosticIgnoranceComputer.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/UTF8Control.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/BslExpression.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/ServerContext.java
- build.gradle.kts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/LanguageClientAwareAppender.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DataExchangeLoadingDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingThisFormDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/MeasuresAspect.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/metadata/DiagnosticInfo.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingExternalCodeToolsDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseOfStrTemplateDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ErrorExpressionNode.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphWalker.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicDateDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/variable/VariableDescription.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseLikeInQueryDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/DiagnosticHelper.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FieldsFromJoinsWithoutIsNullDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DuplicatedInsertionIntoCollectionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java
**/diagnostics/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DataExchangeLoadingDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingThisFormDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingExternalCodeToolsDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseOfStrTemplateDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicDateDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseLikeInQueryDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FieldsFromJoinsWithoutIsNullDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DuplicatedInsertionIntoCollectionDiagnostic.java
🧠 Learnings (12)
📓 Common learnings
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/*.java : Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/diagnostics/*.java : Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DataExchangeLoadingDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/metadata/DiagnosticInfo.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingExternalCodeToolsDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicDateDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/DiagnosticHelper.java
📚 Learning: 2025-04-18T22:46:43.245Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:192-203
Timestamp: 2025-04-18T22:46:43.245Z
Learning: В проекте bsl-language-server класс BSLParser.ExpressionContext наследуется от BSLParserRuleContext, а не напрямую от ParserRuleContext. При работе с ним нужно учитывать специфичные методы BSLParserRuleContext.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DataExchangeLoadingDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseOfStrTemplateDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AbstractMagicValueDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FieldsFromJoinsWithoutIsNullDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DuplicatedInsertionIntoCollectionDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java
📚 Learning: 2025-11-19T09:02:03.154Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3610
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/QueryPackageFoldingRangeSupplier.java:26-26
Timestamp: 2025-11-19T09:02:03.154Z
Learning: В проекте bsl-language-server используется кастомная версия ANTLR4 от 1c-syntax (io.github.1c-syntax:antlr4), которая включает базовый класс org.antlr.v4.runtime.Tokenizer с методами getAst() и getTokens(). SDBLTokenizer и BSLTokenizer наследуются от этого базового класса.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseOfStrTemplateDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/variable/VariableDescription.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseLikeInQueryDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/DiagnosticHelper.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FieldsFromJoinsWithoutIsNullDiagnostic.java
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to docs/*/diagnostics/**/*.md : Update diagnostic documentation in both Russian and English with examples of problematic code and fixes
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingExternalCodeToolsDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/DiagnosticHelper.java
📚 Learning: 2025-02-10T17:13:41.452Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3408
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ProtectedModuleDiagnostic.java:54-58
Timestamp: 2025-02-10T17:13:41.452Z
Learning: In ProtectedModuleDiagnostic.java, the usage of `Range` field as a diagnostic target is accepted as is, despite being marked as a temporary solution in the comments.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseOfStrTemplateDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DuplicatedInsertionIntoCollectionDiagnostic.java
📚 Learning: 2024-07-04T19:35:26.747Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3308
File: src/test/resources/diagnostics/DoubleNegativesDiagnostic.bsl:20-21
Timestamp: 2024-07-04T19:35:26.747Z
Learning: The file `DoubleNegativesDiagnostic.bsl` is a test resource intended to demonstrate the functionality of the DoubleNegatives diagnostic.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java
📚 Learning: 2025-04-18T22:42:30.042Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:88-97
Timestamp: 2025-04-18T22:42:30.042Z
Learning: В реализации ANTLR, если метод `getChildCount()` возвращает значение больше 0, то поле `children` гарантированно не равно null.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ErrorExpressionNode.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java
📚 Learning: 2025-01-19T21:34:39.797Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T21:34:39.797Z
Learning: In BSL Language Server configuration classes, immutable collections (Set.of, List.of) should be used for default values, while mutability is achieved through setters generated by Data annotation.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphWalker.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Applies to **/diagnostics/*Test.java : Write comprehensive unit tests for each diagnostic including test cases for edge cases, following existing test patterns
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicDateDiagnostic.java
📚 Learning: 2025-02-10T17:12:56.150Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3408
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ProtectedModuleDiagnostic.java:63-66
Timestamp: 2025-02-10T17:12:56.150Z
Learning: In BSL Language Server, `documentContext.getServerContext().getConfiguration()` is guaranteed to return a non-null value, making null checks unnecessary.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/DiagnosticHelper.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java
📚 Learning: 2025-01-19T20:47:40.061Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T20:47:40.061Z
Learning: Configuration classes in the BSL Language Server project use mutable collections (HashMap, ArrayList) and Data annotation from Lombok, allowing for modification of configuration properties after initialization.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/DiagnosticHelper.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build
🔇 Additional comments (28)
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/LanguageClientAwareAppender.java (1)
24-33: Nullable import adjustment is correctThe
org.jspecify.annotations.Nullableimport matches the@Nullableusage onclientHolderand the reordering has no behavioral impact. Looks good as-is.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseLikeInQueryDiagnostic.java (1)
34-34: LGTM! Import reordering aligns with PR refactoring goals.The repositioning of the
@Nullableimport is purely cosmetic and consistent with the broader refactoring effort across the codebase.src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/MeasuresAspect.java (2)
37-38: Nullability import is consistent and scoped correctlyAdding
org.jspecify.annotations.Nullableis appropriate here and is only used for theMeasureCollectorfield; there’s no unused-import issue or scope bleed.
53-59:@NullableonmeasureCollectormatches existing guards and lifecycleMarking
measureCollectoras@Nullablealigns with the existingmeasureCollector == nullchecks in all advice methods and withdestroy()explicitly clearing the reference viasetMeasureCollector(null). This makes the optional dependency explicit without changing behavior, and keeps Lombok-based setter injection intact. Based on learnings, this is consistent with the project’s Lombok usage recommendations.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingExternalCodeToolsDiagnostic.java (2)
34-34: Appropriate nullability annotation library.Using
org.jspecify.annotations.Nullableis a good choice for modern Java nullability annotations, providing standardized null-safety semantics.
72-81: Well-applied nullability annotation with proper null-safety handling.The
@Nullableannotation accurately reflects thatctx.IDENTIFIER()can return null in ANTLR-generated parsers. The existing null check on line 73 properly guards against NPE before using the identifier. The diagnostic logic is sound.src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/DiagnosticHelper.java (4)
30-40: LGTM!Import reorganization is correct. All imported types (
ParseTree,TerminalNode,Tree,@Nullable) are actively used in the file, and the jspecify@Nullableannotation aligns with the project's nullability annotation strategy.
156-180: Good nullability annotation.The
@Nullableannotation correctly documents the existing null-safe behavior since the method already checks fornullat line 157. This is a good API clarification.
189-200: Consider nullability consistency with the two-parameter overload.The two-parameter overload now accepts
@Nullable Map<String, Object> configuration, but this three-parameter overload does not annotate itsconfigurationparameter. If a nullconfigurationis passed here, line 194 would throw NPE onconfiguration.containsKey(name).If this is intentional (callers must pass non-null here), consider adding
@NonNullannotation explicitly for clarity. Otherwise, add a null guard for consistency.
55-57: Minor JavaDoc formatting adjustment.Whitespace alignment in JavaDoc is acceptable.
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/UsingThisFormDiagnostic.java (1)
93-93: LGTM! Type inference simplifies the code without sacrificing clarity.The use of
varhere is appropriate since the typeBSLParser.ParamListContextis explicitly present in the method call as a parameter (getRuleContext(BSLParser.ParamListContext.class, 0)), making the type inference clear and unambiguous.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/FieldsFromJoinsWithoutIsNullDiagnostic.java (1)
36-36: LGTM — migration to jspecify nullability annotations.Using
org.jspecify.annotations.Nullableis a good choice as jspecify is the emerging standard for nullability annotations in Java.src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ErrorExpressionNode.java (1)
29-29: LGTM! Nullable annotation correctly documents existing behavior.The
@Nullableannotation onsourceCodeOperatoris appropriate since:
- The no-arg constructor already implies null is acceptable
- Error nodes represent parse failures where source context may be unavailable
- Aligns with the broader nullability annotations being added to
BslExpressionAlso applies to: 47-49
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java (3)
60-60: LGTM! Nullable annotations correctly document field behavior.The field is uninitialized (null by default) and the getter now properly reflects this in its contract.
Also applies to: 78-80
108-116: Clean refactor using pattern matchinginstanceof.Good use of Java 17 pattern matching for type checks. The code is now more concise and readable compared to separate
instanceof+ cast operations.
194-201: LGTM! Nullable parameter is properly handled.The method correctly guards against null
expressionat line 198-201 by pushing anErrorExpressionNode, which aligns with the newly nullableErrorExpressionNodeconstructor.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/metadata/DiagnosticInfo.java (1)
33-33: Import reorder is OK; no functional impactMoving the
org.jspecify.annotations.Nullableimport under the IOUtils import is stylistic only and does not affect behavior. The file remains consistent with the existing Lombok-based style.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/IncorrectUseOfStrTemplateDiagnostic.java (1)
32-32: Import reorder is OK
org.jspecify.annotations.Nullableremains in the correct import group afterParserRuleContext; this is a no-op change that keeps style consistent and doesn’t affect behavior.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MagicDateDiagnostic.java (3)
31-39: LGTM! Import organization improved.The addition of
@Nullableimport and reordering of existing imports follow the coding guidelines for optimizing imports before committing.
175-178: LGTM! Null handling is correct.The
@Nullableannotation is appropriate, and the method correctly guards against null input by returningfalseearly (lines 176-178). This is consistent with the defensive programming pattern used throughout the codebase.
122-126: No action required—null safety is properly implemented.The
@Nullableannotation on theexpressionContextparameter is correct and safe. All three delegated helper methods (insideSimpleAssignment,insideReturnStatement, andinsideAssignmentWithDateMethodForSimpleDate) safely handle null parameters. BothinsideSimpleAssignmentandinsideReturnStatementdelegate toinsideContext()inAbstractMagicValueDiagnostic.java(line 713), which explicitly checks for null at line 715-716 and returnsfalserather than dereferencing a null value.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DoubleNegativesDiagnostic.java (1)
35-36: Import ofjava.util.Objectsis appropriate and scoped correctly
Objectsis used only forrequireNonNullin this class, and the import keeps usage clear and localized. No further action needed here.src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/variable/VariableDescription.java (1)
99-107: Consider removing unused constructor or clarifying its intended purposeThe constructor
VariableDescription(ParameterDescription param)at lines 99-107 is not called anywhere in the codebase. If this constructor is dead code, it should be removed. If it's intended for future use, add a comment explaining the purpose and consider restoring explicit null-safety (e.g.,requireNonNull(param, "param")) since the removal of@NonNullleaves the contract ambiguous—param.link()on line 105 will still NPE if called with null, just without a clear contract.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DataExchangeLoadingDiagnostic.java (1)
37-37: Nullability import matches ongoing JSpecify adoptionThe added
org.jspecify.annotations.Nullableimport is consistent with the null-safety refactor in diagnostics and keeps this class aligned with the rest of the codebase; no issues here.Please ensure
gradlew precommitstill passes JSpecify/nullness checks after this change when you run it for the PR.src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/DuplicatedInsertionIntoCollectionDiagnostic.java (1)
41-41: Импортorg.jspecify.annotations.Nullableвыглядит корректноИмпорт соответствует используемым в классе аннотациям
@Nullableи вписывается в порядок импортов (сперва сторонние библиотеки, затемjava.*). Функционального поведения изменение не затрагивает.При последующем запуске
./gradlew precommitубедитесь, что модуль успешно собирается сjspecifyв classpath и нет предупреждений от анализаторов по дублюющимся аннотациям nullability.src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/MultilingualStringAnalyser.java (1)
27-27: Nullability contracts andvarusage are consistent and safe here
- Marking
variableNameandgetVariableName(...)as@Nullablematches the actual control flow inparse/istVariableUsingInTemplateand removes implicit assumptions.- Annotating
getCodeBlock()as returning a nullableCodeBlockContextis correct given the ancestor lookup and is guarded by explicit null checks before use.- The switch to
var lValue = assignment.lValue();keeps the logic unchanged and improves readability given Java 17 target.Looks good; no further changes needed in this class.
Also applies to: 68-69, 108-119, 200-205, 211-221, 223-225
src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/LanguageServerConfiguration.java (1)
47-47: Nullable return ofgetCustomConfigurationRootis now correctly specified and implemented
- The
@Nullableon the return type matches the control flow:nullis only returned when a configured root exists but lies outsidesrcDir.- The updated Javadoc explicitly documents the nullable return and the condition under which it occurs, which is required for public APIs.
- The internal logic (using
Absolute.pathandstartsWithfor containment) is consistent with the comment and previous intent.No further changes needed here.
Also applies to: 161-185
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/sentry/PermissionFilterBeforeSendCallback.java (1)
40-41: Nullable contracts forexecuteandwaitForPermissionalign with existing usage
- Declaring
executeas returning@Nullable SentryEventmatches the pattern of returning the event only whensendToSentry()is true and otherwise dropping it by returning null.waitForPermissionis correctly marked@Nullableand its result is already handled viaOptional.ofNullable(answerItem), so the annotation simply documents existing behavior.- Removing parameter-level nullability annotations while relying on jspecify for return nullability keeps the implementation straightforward without changing logic.
No issues spotted in this refactor.
Also applies to: 72-79, 93-103, 134-147
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.java
Outdated
Show resolved
Hide resolved
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/SentryAspect.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java (1)
80-99: Fix comment inversion on line 86 and address test failure from exception type changeLine 86 comment says "если нет в кеше, загружаем из valueLoader" but the condition
value != nullis a cache hit that returns immediately. The comment should say "если есть в кеше, возвращаем значение" (if in cache, return value).Additionally,
put()now throwsIllegalArgumentExceptionfor null values (line 104), but the testtestPutNullValue()expectsNullPointerException. This test will fail with the current implementation.Regarding the class JavaDoc claim "потокобезопасный доступ через get(Object, Callable)" (line 40): the current implementation is not atomic per key—multiple concurrent threads accessing the same key can each execute
valueLoaderand race onput(). If per-key atomicity is required, use Ehcache's atomic operations or per-key locking.- if (value != null) { // если нет в кеше, загружаем из valueLoader + if (value != null) { // если есть в кеше, возвращаем значение return (T) value; }
🧹 Nitpick comments (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java (1)
68-77: Comment reformat + whitespace: OK; consider extracting boolean for readability (optional).
The multi-line comment improves readability; condition remains the same. If this condition keeps growing, consider naming sub-conditions (isVirtualTableColumn,isNestedColumn) to keepifreadable.src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java (1)
296-326:var node = ...refactor is safe; consider guarding the cast if non-ParserRuleContext children are possible.
Right nowchildis cast toParserRuleContextin theelsebranch. IfTrees.getChildren(tree)can ever include non-TerminalNodechildren that are notParserRuleContext, this will throwClassCastException. If that can’t happen for this tree shape, fine; otherwise considerelse if (child instanceof ParserRuleContext ctx) { ... }.- } else { - var node = findTerminalNodeContainsPosition((ParserRuleContext) child, position); + } else if (child instanceof ParserRuleContext childCtx) { + var node = findTerminalNodeContainsPosition(childCtx, position); if (node.isPresent()) { return node; } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java(3 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/aop/EventPublisherAspect.java
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.java
**/diagnostics/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Each diagnostic should have a Java implementation class, resource bundle for localized messages, unit tests, and documentation
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java
🧠 Learnings (4)
📚 Learning: 2025-11-19T09:02:03.154Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3610
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/QueryPackageFoldingRangeSupplier.java:26-26
Timestamp: 2025-11-19T09:02:03.154Z
Learning: В проекте bsl-language-server используется кастомная версия ANTLR4 от 1c-syntax (io.github.1c-syntax:antlr4), которая включает базовый класс org.antlr.v4.runtime.Tokenizer с методами getAst() и getTokens(). SDBLTokenizer и BSLTokenizer наследуются от этого базового класса.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java
📚 Learning: 2025-04-18T22:46:43.245Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:192-203
Timestamp: 2025-04-18T22:46:43.245Z
Learning: В проекте bsl-language-server класс BSLParser.ExpressionContext наследуется от BSLParserRuleContext, а не напрямую от ParserRuleContext. При работе с ним нужно учитывать специфичные методы BSLParserRuleContext.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java
📚 Learning: 2025-01-20T19:33:59.692Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/codelenses/RunAllTestsCodeLensSupplier.java:55-59
Timestamp: 2025-01-20T19:33:59.692Z
Learning: In Spring, self-injection pattern (using Autowired Lazy on a self-reference field) is required to enable caching through Cacheable when methods are called from within the same class, as Spring's proxy-based AOP doesn't work with direct 'this' references.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
📚 Learning: 2025-01-19T20:47:40.061Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T20:47:40.061Z
Learning: Configuration classes in the BSL Language Server project use mutable collections (HashMap, ArrayList) and Data annotation from Lombok, allowing for modification of configuration properties after initialization.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: Benchmark
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build
- GitHub Check: Analyse
🔇 Additional comments (5)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/QueryNestedFieldsByDotDiagnostic.java (1)
58-64: Cosmetic change only; logic preserved.
Space in theifcondition is fine and aligns with the style guide expectations.src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java (2)
33-40: Import of@Nullable/Collectionslooks fine; keep consistent project-wide nullability convention.
No issues here; matches PR direction.
97-102: Document the null-handling contract in JavaDoc.The method already safely handles
nullby returning an empty list, which works correctly with all call sites (they useaddAll(), for-each loops, and similar idempotent operations). However, the JavaDoc does not document this contract—add a note clarifying thatnullinput returns an empty list so callers understand the intended behavior.src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java (2)
24-24:org.jspecify.annotations.Nullableimport is fine—ensure tooling is configured
Just make sure JSpecify nullness checking is actually enabled/consistent in this project (IDE + build), otherwise annotations become documentation-only.
101-112: Fix spacing in if statement and verify exception type against test expectationsThe style issue is correct—replace
if(value == null)withif (value == null)following standard Java conventions (space after control flow keywords). However, the exception type change fromIllegalArgumentExceptiontoNullPointerExceptioncannot be fully verified because the referenced test fileEhCacheAdapterTest.testPutNullValue()could not be located in this repository. The review comment references a "previous review thread" that explicitly expectedNullPointerException, but without access to that test or the previous discussion, the exception type change should be verified against actual test expectations and Spring Framework's Cache contract before implementation. Standard Spring caching implementations often enforce this pattern, but confirmation is needed.@Override public void put(Object key, @Nullable Object value) { - if(value == null) { // не будем в кеш класть null-значения - throw new IllegalArgumentException("Null values are not allowed for cache \"" + name + "\""); + if (value == null) { // не будем в кеш класть null-значения + throw new IllegalArgumentException("Null values are not allowed for cache \"" + name + "\""); }
src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraph.java
Show resolved
Hide resolved
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 675c8e3. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
.github/workflows/gradle.yml (1)
21-22:actions/download-artifact@v6does not exist—use@v4insteadLine 67:
actions/download-artifact@v6must be replaced with@v4, which is the current supported GA release. Version 6 of this action does not exist on the GitHub Actions Marketplace.- uses: actions/download-artifact@v6 + uses: actions/download-artifact@v4Note:
actions/checkout@v6(lines 21, 39) is correct and reflects the latest version.src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java (2)
101-108: Theput(key, @Nullable value)method currently allows null values to reach the native cache, violating the adapter's contract — the JavaDoc explicitly states "Не допускает хранение null-значений" and the constructor passesfalsetosuper(), indicatingallowNullValues = false. The testtestPutNullValue()expects aNullPointerExceptionto be thrown.Add a null check before casting to ensure fail-fast behavior consistent with the
get(key, Callable)method (line 92):@Override public void put(Object key, @Nullable Object value) { + if (value == null) { + throw new NullPointerException("Null values are not allowed for cache \"" + name + "\""); + } @SuppressWarnings("unchecked") var typedKey = (K) key; @SuppressWarnings("unchecked") var typedValue = (V) value; nativeCache.put(typedKey, typedValue); }
24-28: Theput()method must reject null values to match its declared contract and test expectations.The class documents that it does not allow null values (
allowNullValues = false), and the test at line 189–192 (testPutNullValue()) explicitly expectsput("key1", null)to throw aNullPointerException. However, the current implementation at lines 102–108 accepts the@Nullableparameter and passes it directly to the underlying cache without validation, allowing null to be stored in violation of the contract. Add a null check to throw early:@Override public void put(Object key, @Nullable Object value) { if (value == null) { throw new NullPointerException("Null values are not allowed"); } @SuppressWarnings("unchecked") var typedKey = (K) key; @SuppressWarnings("unchecked") var typedValue = (V) value; nativeCache.put(typedKey, typedValue); }Also update the parameter annotation from
@Nullableto@NonNullto accurately reflect the actual contract.Minor: The comment at line 86 is misleading. Change
if (value != null) { // если нет в кеше, загружаем из valueLoaderto reflect that the block executes when a value IS found in the cache, not when it's absent.The
org.jspecify.annotations.Nullableimport is appropriate for the project and compatible with Java 17 and Spring Framework.
🧹 Nitpick comments (3)
.github/workflows/gradle.yml (3)
59-64: Tighten job permissions (drop unusedissues: read, keep least-privilege)
Unless you’re intentionally reading issues in this job,issues: readis extra surface area.permissions: checks: write pull-requests: write contents: read - issues: read
12-12: Add parentheses toif:for unambiguous precedence
Current expression (Line 12) mixes&&and||; it likely works as intended but is easy to regress.- if: github.event_name == 'pull_request' && startsWith(github.head_ref, 'translations_') == false || github.event_name == 'push' + if: (github.event_name == 'pull_request' && startsWith(github.head_ref, 'translations_') == false) || github.event_name == 'push'
70-73: Switch deprecatedjunit_filesinput tofilesThe
EnricoMi/publish-unit-test-result-action@v2action has deprecated thejunit_filesinput parameter. Usefilesinstead to maintain compatibility.- name: Publish Test Results uses: EnricoMi/publish-unit-test-result-action@v2 with: - junit_files: "artifacts/**/*.xml" + files: "artifacts/**/*.xml"
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/gradle.yml(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
🧠 Learnings (3)
📚 Learning: 2025-11-27T07:17:33.726Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-27T07:17:33.726Z
Learning: Always run tests before submitting changes using ./gradlew test and maintain or improve test coverage
Applied to files:
.github/workflows/gradle.yml
📚 Learning: 2025-01-20T19:33:59.692Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/codelenses/RunAllTestsCodeLensSupplier.java:55-59
Timestamp: 2025-01-20T19:33:59.692Z
Learning: In Spring, self-injection pattern (using Autowired Lazy on a self-reference field) is required to enable caching through Cacheable when methods are called from within the same class, as Spring's proxy-based AOP doesn't work with direct 'this' references.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
📚 Learning: 2025-01-19T20:47:40.061Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T20:47:40.061Z
Learning: Configuration classes in the BSL Language Server project use mutable collections (HashMap, ArrayList) and Data annotation from Lombok, allowing for modification of configuration properties after initialization.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
🪛 actionlint (1.7.9)
.github/workflows/gradle.yml
73-73: avoid using deprecated input "junit_files" in action "EnricoMi/publish-unit-test-result-action@v2": Use "files" option instead
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Analyse
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build
src/main/java/com/github/_1c_syntax/bsl/languageserver/infrastructure/EhCacheAdapter.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java (1)
110-116: LGTM! Clean refactor using pattern matching.The use of pattern-matching
instanceofwith binding variables improves readability and eliminates explicit casts. ThePreprocessorContextcase correctly skips processing via the inline comment.Minor: For consistency and clarity, consider making the no-op explicit:
} else if (child instanceof BSLParser.PreprocessorContext) { - // просто пропускаем + // просто пропускаем - no operation needed + continue; } else {
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/gradle.yml(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java(6 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java
🧠 Learnings (2)
📚 Learning: 2025-04-18T22:46:43.245Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:192-203
Timestamp: 2025-04-18T22:46:43.245Z
Learning: В проекте bsl-language-server класс BSLParser.ExpressionContext наследуется от BSLParserRuleContext, а не напрямую от ParserRuleContext. При работе с ним нужно учитывать специфичные методы BSLParserRuleContext.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java
📚 Learning: 2025-04-18T22:42:30.042Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3449
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java:88-97
Timestamp: 2025-04-18T22:42:30.042Z
Learning: В реализации ANTLR, если метод `getChildCount()` возвращает значение больше 0, то поле `children` гарантированно не равно null.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java
🪛 actionlint (1.7.9)
.github/workflows/gradle.yml
73-73: avoid using deprecated input "junit_files" in action "EnricoMi/publish-unit-test-result-action@v2": Use "files" option instead
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build
- GitHub Check: Analyse
🔇 Additional comments (3)
.github/workflows/gradle.yml (1)
50-55: Good practice:if: always()ensures test results are archived on failure.The addition of
if: always()to the Archive test results step is solid—it guarantees test artifacts are collected regardless of build success, which is critical for CI observability. The updated artifact name and path are also appropriately scoped to isolate test results by Java version and OS.src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java (2)
196-203: LGTM! Proper null safety handling.The
@Nullableannotation on theexpressionparameter is correctly paired with an explicit null check at line 200, ensuring safe handling of empty parentheses cases.
80-82: No API breaking change here—@Nullablecorrectly reflects the field type.The
resultExpressionfield is already declared as@Nullable BslExpression(line 60), so the method's return type is accurate. All callers handle null appropriately: the internal caller usesassert result != null(line 73), andAbstractExpressionTreeDiagnosticexplicitly checksif (expressionTree != null)before use (line 115). The annotation is correct documentation of existing behavior, not a breaking change.
| publish-test-results: | ||
| needs: [gatekeeper, check-pr-exists, build] | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| checks: write | ||
| pull-requests: write | ||
| contents: read | ||
| issues: read | ||
| if: always() | ||
| steps: | ||
| - name: Download Artifacts | ||
| uses: actions/download-artifact@v6 | ||
| with: | ||
| path: artifacts | ||
| - name: Publish Test Results | ||
| uses: EnricoMi/publish-unit-test-result-action@v2 | ||
| with: | ||
| junit_files: "artifacts/**/*.xml" |
There was a problem hiding this comment.
Replace deprecated junit_files input with files.
The new publish-test-results job is well-structured with proper permissions and dependencies. However, the action uses a deprecated input parameter.
Apply this diff to use the current action API:
- name: Publish Test Results
uses: EnricoMi/publish-unit-test-result-action@v2
with:
- junit_files: "artifacts/**/*.xml"
+ files: "artifacts/**/*.xml"This aligns with the latest EnricoMi/publish-unit-test-result-action@v2 API and avoids potential breaking changes in future versions.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| publish-test-results: | |
| needs: [gatekeeper, check-pr-exists, build] | |
| runs-on: ubuntu-latest | |
| permissions: | |
| checks: write | |
| pull-requests: write | |
| contents: read | |
| issues: read | |
| if: always() | |
| steps: | |
| - name: Download Artifacts | |
| uses: actions/download-artifact@v6 | |
| with: | |
| path: artifacts | |
| - name: Publish Test Results | |
| uses: EnricoMi/publish-unit-test-result-action@v2 | |
| with: | |
| junit_files: "artifacts/**/*.xml" | |
| publish-test-results: | |
| needs: [gatekeeper, check-pr-exists, build] | |
| runs-on: ubuntu-latest | |
| permissions: | |
| checks: write | |
| pull-requests: write | |
| contents: read | |
| issues: read | |
| if: always() | |
| steps: | |
| - name: Download Artifacts | |
| uses: actions/download-artifact@v6 | |
| with: | |
| path: artifacts | |
| - name: Publish Test Results | |
| uses: EnricoMi/publish-unit-test-result-action@v2 | |
| with: | |
| files: "artifacts/**/*.xml" |
🧰 Tools
🪛 actionlint (1.7.9)
73-73: avoid using deprecated input "junit_files" in action "EnricoMi/publish-unit-test-result-action@v2": Use "files" option instead
(action)
🤖 Prompt for AI Agents
.github/workflows/gradle.yml around lines 56 to 73: the
publish-unit-test-result-action usage still uses the deprecated input parameter
`junit_files`; update the action call to replace `junit_files:
"artifacts/**/*.xml"` with the current `files: "artifacts/**/*.xml"` input
(preserve the same glob pattern), and verify the action version remains @v2 so
no other input changes are required.
| var result = instance.getExpressionTree(); | ||
| assert result != null; | ||
| return result; |
There was a problem hiding this comment.
Replace assert with proper runtime null check.
Using assert for validating non-null return values is unsafe because assertions can be disabled in production via the -da JVM flag. If disabled, buildExpressionTree() could return null despite its non-null return type, causing NullPointerException downstream.
Apply this diff to use a proper runtime check:
public static BslExpression buildExpressionTree(BSLParser.ExpressionContext ctx) {
var instance = new ExpressionTreeBuildingVisitor();
instance.visitExpression(ctx);
- var result = instance.getExpressionTree();
- assert result != null;
- return result;
+ return Objects.requireNonNull(
+ instance.getExpressionTree(),
+ "Expression tree must not be null after visiting expression context"
+ );
}🤖 Prompt for AI Agents
In
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/expressiontree/ExpressionTreeBuildingVisitor.java
around lines 72 to 74, replace the use of `assert result != null;` with a
runtime null check: if the expression tree is null throw an unchecked exception
(e.g. IllegalStateException or NullPointerException) with a clear message
indicating buildExpressionTree() returned null, otherwise return the result;
this ensures the non-null contract is enforced regardless of JVM assertion
settings.
|
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/symbol/ModuleSymbol.java
Show resolved
Hide resolved
Описание
Исправление замечаний к коду, рефакторинг
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.