nullsafe#560
Conversation
📝 WalkthroughWalkthroughReplaced legacy non-null annotations with JSpecify ( Changes
Sequence Diagram(s)Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
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 |
Test Results 603 files + 54 603 suites +54 8m 33s ⏱️ -25s Results for commit 216646d. ± Comparison against base commit bc9178a. This pull request removes 2 and adds 47 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
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/reader/common/context/std_attributes/StdAtrInfo.java (1)
159-172: The change is correct, but the underlying method signature is incomplete for null-safety migration.The change from
getFromCache("owners", "")togetFromCache("owners")correctly usesnullto represent a cache miss, which is semantically more accurate than an empty string sentinel. Theinstanceofchecks safely handlenull(returningfalse), so behavior is preserved with no NPE risk.However, the
AbstractReaderContext.getFromCache(String key)method (line 196) is missing an explicit@Nullableannotation on its return type. Under the package-level@NullMarkeddeclaration, the return type should be annotated as@Nullable Tsincecache.get()can returnnull. Update the method signature to:@SuppressWarnings("unchecked") public <T> @Nullable T getFromCache(String key) { return (T) cache.get(key.toLowerCase(Locale.ROOT)); }This ensures consistent null-safety across the JSpecify migration throughout the codebase.
src/main/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtils.java (1)
112-122: Critical issue:FormHandlerlacks a builder method and will causeIllegalArgumentExceptionat runtime.The
builder()method now throwsIllegalArgumentExceptionwhen the class lacks a builder method (previously returnednull). However,FormElementReaderContext.CLASSESmap includesFormHandler.class(mapped to "Events"), butFormHandleris a record without@Builderannotation. When processing form events,TransformationUtils.builder(FormHandler.class)will throw an exception, causing parsing failures.Either add
@BuildertoFormHandler, remove it from theCLASSESmap, or revertbuilder()to returnnullto maintain backward compatibility.
🧹 Nitpick comments (4)
src/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.java (1)
125-129: LGTM! Clean refactor to switch expression.The migration from if-else to switch expression improves readability while preserving the original logic.
src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MemoryProfiler.java (1)
66-71: LGTM! Excellent covariance refinement improves type safety.The return type change from
Collection<Result>toCollection<? extends Result>with the narrowed local variableList<ScalarResult>correctly reflects that this method exclusively producesScalarResultinstances. This improves type precision without breaking the interface contract.Also applies to: 118-118
src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java (2)
245-247: Consider replacingassertwith explicit null check for reliability.The
assertstatement provides no runtime guarantee when assertions are disabled (default in production). IfTransformationUtils.build()returns null due to a reflection failure, this will cause aNullPointerExceptionat the cast on line 247 rather than a clear error.🔎 Proposed fix using Objects.requireNonNull
TransformationUtils.setValue(builder, "modules", merge); - var computeResult = TransformationUtils.build(builder); - assert computeResult != null; // там и не будет null - return (T) computeResult; + var computeResult = Objects.requireNonNull( + TransformationUtils.build(builder), + "build() returned null for " + srcMD.getClass() + ); + return (T) computeResult;
285-287: Same concern:assertdoes not provide runtime guarantees.Same issue as in
copyModuleOwner- consider usingObjects.requireNonNullfor consistent error handling.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (57)
build.gradle.ktssrc/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MemoryProfiler.javasrc/main/java/com/github/_1c_syntax/bsl/mdclasses/helpers/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdclasses/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/children/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/DataCompositionSchema.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/QuerySource.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/RoleData.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/TemplateData.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/XdtoPackageData.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/form/FormHandler.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/form/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/support/Handler.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/support/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/utils/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/FakeReader.javasrc/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.javasrc/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.javasrc/main/java/com/github/_1c_syntax/bsl/reader/MDReader.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtils.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/AbstractReaderContext.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/FormElementReaderContext.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDCReaderContext.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDReaderContext.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/StdAtrInfo.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/CommonAttributeUseContentConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/EnumConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/ExchangePlanAutoRecordConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/RoleDataConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/SubsystemConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/ValueTypeConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/XdtoPackageDataConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/xstream/ExtendXStream.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/xstream/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/FormAttributeConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/FormElementConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MDChildConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MdoReferenceConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/TemplateConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/Unmarshaller.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/EDTReader.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/ExternalDataSourceConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/FormHandlerConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/FormItemConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/MdoReferenceConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/package-info.java
💤 Files with no reviewable changes (1)
- src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/XdtoPackageDataConverter.java
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: theshadowco
Repo: 1c-syntax/mdclasses PR: 524
File: src/main/java/com/github/_1c_syntax/bsl/mdo/support/UsePurposes.java:61-69
Timestamp: 2025-10-06T14:38:27.354Z
Learning: В проекте mdclasses используются JSR-305 аннотации (ParametersAreNonnullByDefault) на уровне пакетов для обеспечения null-safety через статический анализ, что является приемлемым подходом вместо явных runtime проверок.
📚 Learning: 2025-10-06T14:38:27.354Z
Learnt from: theshadowco
Repo: 1c-syntax/mdclasses PR: 524
File: src/main/java/com/github/_1c_syntax/bsl/mdo/support/UsePurposes.java:61-69
Timestamp: 2025-10-06T14:38:27.354Z
Learning: В проекте mdclasses используются JSR-305 аннотации (ParametersAreNonnullByDefault) на уровне пакетов для обеспечения null-safety через статический анализ, что является приемлемым подходом вместо явных runtime проверок.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/TemplateData.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MdoReferenceConverter.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/children/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MDChildConverter.javasrc/main/java/com/github/_1c_syntax/bsl/mdclasses/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/utils/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/form/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/form/FormHandler.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDCReaderContext.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/storage/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/support/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.javasrc/main/java/com/github/_1c_syntax/bsl/mdclasses/helpers/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/mdo/support/Handler.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/MdoReferenceConverter.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtils.javasrc/main/java/com/github/_1c_syntax/bsl/reader/designer/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/xstream/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/MDReader.javasrc/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDReaderContext.javasrc/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java
📚 Learning: 2025-10-06T15:29:06.086Z
Learnt from: theshadowco
Repo: 1c-syntax/mdclasses PR: 0
File: :0-0
Timestamp: 2025-10-06T15:29:06.086Z
Learning: В проекте mdclasses принято писать Javadoc на русском языке.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/mdclasses/package-info.java
📚 Learning: 2025-09-05T10:03:02.229Z
Learnt from: johnnyshut
Repo: 1c-syntax/mdclasses PR: 515
File: src/test/java/com/github/_1c_syntax/bsl/mdo/AccountingRegisterTest.java:35-37
Timestamp: 2025-09-05T10:03:02.229Z
Learning: The MDTestUtils.getMDWithSimpleTest method already correctly handles optional 4th parameter "_edt" through ArgumentsAccessor - it doesn't need modification to process the additional CSV parameter for subdirectory selection.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/reader/edt/EDTReader.javasrc/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.java
📚 Learning: 2025-10-15T07:02:30.766Z
Learnt from: theshadowco
Repo: 1c-syntax/mdclasses PR: 533
File: src/test/java/com/github/_1c_syntax/bsl/mdclasses/MDClassesTest.java:0-0
Timestamp: 2025-10-15T07:02:30.766Z
Learning: For EDT format external sources in mdclasses tests, the actual external source count is determined by counting individual items within src/test/resources/ext/edt/external/src/ExternalDataProcessors and src/test/resources/ext/edt/external/src/ExternalReports directories, not by counting the parent directories themselves.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.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). (16)
- GitHub Check: QA
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (17, windows-latest)
🔇 Additional comments (81)
src/main/java/com/github/_1c_syntax/bsl/reader/FakeReader.java (2)
39-39: LGTM!Import correctly migrated from
javax.annotation.Nullableto JSpecify'sorg.jspecify.annotations.Nullable, consistent with the null-safe migration objective. Based on learnings, this project uses package-level nullability annotations for static analysis.
85-88: Behavioral change:getXstream()now throws instead of returning null.This method previously returned
null(with@Nullable), but now throwsIllegalCallerException. While appropriate for a stub, this is a breaking change. Both known usages (ExtendXStream.java:156andMDReaderContext.java:91) immediately invoke methods on the returned value. Verify thatFakeReaderinstances cannot reach these code paths, or confirm callers are prepared to handle the exception.src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/FormHandlerConverter.java (2)
1-22: LGTM!Standard copyright year update.
28-28: Correct null-safety annotation.The
@Nullableannotation accurately documents thatunmarshalreturns null when both event and name are empty (line 56-57). This aligns with the PR's migration to JSpecify annotations. Package-level@NullMarkedannotation is present inpackage-info.java, and theReadConverterinterface contract permits nullable return values through method-level@Nullableoverride of the package-level default.src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/package-info.java (1)
25-28: Standard JSpecify package-level null-marking.The migration from JSR-305 to JSpecify's
@NullMarkedat the package level is correctly implemented and aligns with the project's established approach of using package-level annotations for null-safety.Based on learnings, the project previously used JSR-305 annotations (ParametersAreNonnullByDefault) at the package level for null-safety through static analysis. This JSpecify migration maintains that pattern with modern annotations.
src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/FormItemConverter.java (1)
31-31: @nullable annotation onunmarshal()is correctly applied.The package is marked with
@NullMarked, making all types non-null by default. The@Nullableannotation is necessary here because the method explicitly returnsnull(line 43), which deviates from the package's null-safety defaults. The implementation is sound.src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/ExternalDataSourceConverter.java (1)
53-54: LGTM: Nullability contract properly documented.The
@Nullableannotation correctly indicates that this method may return null (e.g., via thesuper.read()call at line 74). This improves the nullability contract for callers.src/main/java/com/github/_1c_syntax/bsl/mdo/storage/XdtoPackageData.java (2)
40-44: Migration to @NullMarked is correctly implemented.The removal of
@NonNullannotations aligns with the project-wide migration to JSpecify's@NullMarkedat package level, where unannotated types default to non-null. The package-level@NullMarkedannotation is properly configured inpackage-info.java, and theEMPTYconstant correctly uses an empty string and empty lists rather than null values, consistent with the non-null contract.
61-64: The fieldsbase,variety,type, andformcannot be null at runtime. TheXdtoPackageDataConverter.getAttribute()method (lines 167–173) explicitly converts missing XML attributes to empty strings rather than null, ensuring these String fields are always non-null. No@Nullableannotations are required.Likely an incorrect or invalid review comment.
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MdoReferenceConverter.java (2)
28-28: LGTM! Nullability annotation correctly reflects return behavior.The
@Nullableannotation accurately documents thatunmarshalreturnsnullfor empty values or values containing "-" (line 53). This is appropriate for a deserialization converter handling invalid references.Also applies to: 43-56
22-22: Package-level null-safety configuration is correctly in place.The package
com.github._1c_syntax.bsl.reader.designer.converterhas the@NullMarkedannotation inpackage-info.java, and theMdoReferenceConverter.unmarshalmethod correctly declares@Nullableon its return type, which is compatible with the interface contract and consistent with the method's implementation.src/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.java (4)
33-33: Good addition of JSpecify import.The import aligns with the project's migration to standardized null-safety annotations.
71-93: LGTM! Correct use of@Nullableannotations.The
@Nullableannotations and updated Javadoc correctly document that these methods may return null when reading fails.
132-158: LGTM! Good defensive null handling.The addition of
@Nullableparameter annotation and early null check improves null-safety without changing the core logic.
160-178: LGTM! Improved null-safety and validation.The addition of
@Nullableparameter annotation, null check, and file existence validation are good defensive improvements. The early-return pattern also enhances readability.src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/TemplateConverter.java (3)
35-35: LGTM: JSpecify import added for null-safe migration.The import aligns with the PR's objective of migrating to JSpecify annotations.
46-46: LGTM: @nullable annotation correctly documents nullable return.The annotation properly indicates that
unmarshalcan return null, which aligns with the new null check at line 52-54.
52-54: Fix style inconsistency: missing space afterifkeyword.The null check at lines 52–54 is necessary and correctly prevents potential NPEs on lines 56 and 74 where
realClass.isAssignableFrom()is called. The method is properly annotated with@Nullable, and returning null is a standard pattern used consistently throughout the codebase's XStream converters (seeFormElementConverter,FormAttributeConverter,MdoReferenceConverter, and others).Style issue: Line 52 is missing a space after
if. All other conditional statements in the file (lines 56, 58, 66, 74) consistently useif (with a space.- if(realClass == null) { + if (realClass == null) {src/main/java/com/github/_1c_syntax/bsl/mdo/storage/RoleData.java (1)
41-41: No action needed — package is properly annotated with@NullMarked.The package
com.github._1c_syntax.bsl.mdo.storageis correctly annotated with@NullMarked(JSpecify). The removal of@NonNullfromobjectRights,ObjectRight.name,ObjectRight.rights, andRight.nameis safe and part of the proper null-safety migration. With package-level@NullMarked, all non-annotated types default to non-null.src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/SubsystemConverter.java (2)
34-34: LGTM! Import aligns with JSpecify migration.The import of
@Nullablefrom JSpecify is appropriate for the null-safety migration described in the PR objectives.
54-78: Verify interface contract consistency for nullability annotations.The
@Nullableannotation is appropriate sinceExtendXStream.read()can return null (line 61). However:
- The
ReadConverter.unmarshal()method inherited from XStream'sConverterinterface likely lacks@Nullable, which creates a contract mismatch when the implementation declares@Nullable- The method's other return paths (lines 73, 75) are guaranteed non-null:
readerContextis a newly constructed object, andreaderContext.build()includes anassertensuring non-null returnConsider whether the entire method should be
@Nullableor if the implementation should handle the potential null from line 61 explicitly to avoid declaring a nullable method when most paths are non-null.src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/MDChildConverter.java (2)
38-38: LGTM! Null-safety migration aligns with PR objectives.The addition of the JSpecify
@Nullableannotation on theunmarshalmethod correctly documents the nullability contract, which is consistent with the broader migration from legacy annotations to JSpecify across the codebase.Also applies to: 54-54
59-59:andsrc/main/java/com/github/_1c_syntax/bsl/reader/common/converter/ExchangePlanAutoRecordConverter.java (4)
32-32: LGTM! Correct import for JSpecify null-safety.The addition of JSpecify's
@Nullableimport aligns with the PR's objective of migrating to standardized null-safety annotations.
44-44: LGTM! Correctly marks nullable return type.The
@Nullableannotation is justified by the method's ability to return null (line 47) and by returning the nullable result fromexchangePlanAutoRecord(line 61).
70-73: LGTM! Correct type-use annotation for nullable return.The
ExchangePlan.@Nullable RecordContentannotation correctly marks the return type as nullable, justified by the early return at line 75. The type-use annotation syntax is appropriate for the nested class.
53-56: The null check in lines 54-56 correctly prevents null entries from being added to the content list. This is the intended behavior and follows a consistent pattern used across similar converters (e.g., CommonAttributeUseContentConverter). Since theexchangePlanAutoRecord()helper can legitimately return null when there are no child elements to process, and the returnedList<RecordContent>is directly streamed without null-safety checks elsewhere in the codebase, filtering null values is appropriate.src/main/java/com/github/_1c_syntax/bsl/mdo/storage/form/FormHandler.java (1)
26-26: LGTM: Import migration to JSpecify.The import change from FindBugs to JSpecify annotations is part of the systematic nullability annotation migration across the codebase.
src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/package-info.java (1)
25-28: LGTM: Package-level null-safety annotation.The
@NullMarkedannotation establishes package-level non-null defaults, replacing the previous JSR-305 approach with JSpecify's standardized annotations.src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java (1)
76-76: LGTM: Import migration to JSpecify.The import update aligns with the project-wide migration to JSpecify nullability annotations.
src/main/java/com/github/_1c_syntax/bsl/mdo/storage/TemplateData.java (1)
24-25: LGTM: Import migration to JSpecify.The import change is part of the systematic migration to JSpecify annotations.
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/package-info.java (1)
25-28: LGTM: Package-level null-safety annotation.The
@NullMarkedannotation establishes non-null by default semantics for the package, consistent with the JSpecify migration strategy.src/main/java/com/github/_1c_syntax/bsl/mdo/support/Handler.java (1)
25-25: LGTM: Import migration to JSpecify.The import change maintains the same nullability semantics with JSpecify annotations.
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/Unmarshaller.java (1)
139-141: LGTM: Defensive null handling.The null check prevents potential NPE when
fieldClassis used on line 144. This pattern is consistent with the existing null check in thereadNodemethod (lines 150-153) and aligns well with the null-safety improvements in this PR.build.gradle.kts (1)
71-72: JSpecify version 1.0.0 is current. The dependency uses the latest stable release from Maven Central.src/main/java/com/github/_1c_syntax/bsl/reader/common/context/std_attributes/package-info.java (1)
25-28: LGTM! Clean migration to JSpecify nullability annotations.The package-level
@NullMarkedannotation correctly establishes non-null as the default for all types in this package, consistent with the broader migration strategy across the codebase.Based on learnings, the project has historically used package-level null-safety annotations for static analysis, and this migration to JSpecify standardizes that approach.
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/package-info.java (1)
25-28: LGTM! Package-level nullability correctly established.The
@NullMarkedannotation is properly applied, maintaining consistency with the project-wide nullability migration.src/main/java/com/github/_1c_syntax/bsl/reader/edt/converter/MdoReferenceConverter.java (1)
28-28: LGTM! Correct nullability annotation for method that can return null.The
@Nullableannotation accurately reflects thatunmarshalreturnsnullfor invalid input (line 52) and a validMdoReferenceotherwise. This makes the null-returning contract explicit for static analysis and caller awareness.Also applies to: 39-40
src/main/java/com/github/_1c_syntax/bsl/reader/common/package-info.java (1)
25-28: LGTM! Consistent nullability migration.Package-level
@NullMarkedannotation properly applied.src/main/java/com/github/_1c_syntax/bsl/reader/package-info.java (1)
25-28: LGTM! Package nullability correctly configured.src/main/java/com/github/_1c_syntax/bsl/reader/common/xstream/package-info.java (1)
25-28: LGTM! Package-level nullability migration complete.src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/FormAttributeConverter.java (1)
31-31: LGTM! Nullable annotation correctly reflects multiple null-return paths.The
@Nullableannotation onunmarshalaccurately documents that the method returnsnullfor unsupported types (line 46) and parse errors (line 54), making the contract explicit for callers.Also applies to: 42-43
src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MemoryProfiler.java (1)
1-140: No action needed. The repository's GitHub Actions workflow (check.yml) automatically runsgradlew checkon all push and pull request events, which includes test execution, code coverage analysis, and validation. There is no PR checklist indicating precommit actions are pending—the continuous integration pipeline already validates code on every PR submission.Likely an incorrect or invalid review comment.
src/main/java/com/github/_1c_syntax/bsl/mdclasses/package-info.java (1)
27-30: LGTM! Clean migration to JSpecify.The package-level nullability semantics remain equivalent:
@NullMarkedprovides the same non-null defaults for parameters and return values as the previous@ParametersAreNonnullByDefault+@ReturnValuesAreNonnullByDefaultcombination.src/main/java/com/github/_1c_syntax/bsl/mdo/storage/form/package-info.java (1)
25-28: LGTM! Consistent with project-wide JSpecify adoption.src/main/java/com/github/_1c_syntax/bsl/reader/designer/package-info.java (1)
25-28: LGTM!src/main/java/com/github/_1c_syntax/bsl/mdo/package-info.java (1)
25-28: LGTM!src/main/java/com/github/_1c_syntax/bsl/reader/edt/EDTReader.java (1)
74-74: LGTM! Aligned with JSpecify migration.src/main/java/com/github/_1c_syntax/bsl/mdo/utils/package-info.java (1)
25-28: LGTM!src/main/java/com/github/_1c_syntax/bsl/mdo/support/package-info.java (1)
27-30: LGTM!src/main/java/com/github/_1c_syntax/bsl/mdclasses/helpers/package-info.java (1)
25-28: Migration to JSpecify is complete and consistent. All legacy nullability annotations (javax.annotation, FindBugs, Lombok) have been removed from the codebase, the jspecify dependency is properly configured in build.gradle.kts, and @NullMarked is consistently applied at the package level across all packages.src/main/java/com/github/_1c_syntax/bsl/reader/edt/package-info.java (1)
25-28: LGTM!Clean migration to JSpecify
@NullMarked. This single annotation correctly replaces the previous@ParametersAreNonnullByDefaultand@ReturnValuesAreNonnullByDefaultannotations, providing equivalent null-safety semantics at the package level. Based on learnings, this aligns with the project's existing approach of using package-level annotations for null-safety via static analysis.src/main/java/com/github/_1c_syntax/bsl/reader/MDReader.java (1)
35-35: LGTM!Import correctly updated to use JSpecify's
@Nullable. Methods that can return null retain explicit@Nullableannotations, while other methods inherit non-null semantics from the package-level@NullMarkedannotation.src/main/java/com/github/_1c_syntax/bsl/reader/common/xstream/ExtendXStream.java (3)
118-134: LGTM!Good defensive enhancement:
fromXMLnow properly handlesStreamExceptionin addition to the existing exception handling. The@Nullableannotation correctly reflects that this method can return null when the file doesn't exist or is malformed.
142-145: LGTM!The
@Nullableannotation is appropriate sincegetRealClassdelegates to the mapper which may not find a matching class.
154-157: LGTM!Static helper methods correctly annotated as
@Nullableto reflect that underlying read operations may return null.Also applies to: 166-169, 179-182
src/main/java/com/github/_1c_syntax/bsl/reader/designer/converter/FormElementConverter.java (1)
41-57: LGTM!The
@Nullableannotation correctly reflects thatunmarshalcan return null in two scenarios: when form element items should be skipped, or when the ID parsing fails. The logging simplification at line 51 is appropriate.src/main/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtils.java (2)
63-78: LGTM!The
@Nullableannotation forvalueparameter is appropriate. The existing null check on line 65 (value != null) already handles null values correctly.
151-175: LGTM!Both
buildoverloads correctly return@Nullablesince they can return null when method invocation fails. The path-aware overload provides better error context in logging.src/main/java/com/github/_1c_syntax/bsl/reader/MDMerger.java (2)
293-314: LGTM!The
@Nullableannotation and return semantics are correct. The method properly handles the case wherebuilderremains null (no changes needed) and returns it as-is.
60-60: LGTM!The removal of explicit
@NonNullis correct since the package-level@NullMarkednow provides the non-null default for parameters.src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/CommonAttributeUseContentConverter.java (2)
47-56: LGTM!Good defensive programming: the null check before adding to the list prevents potential
NullPointerExceptionwhen iterating. The@Nullableannotation correctly reflects that the method can return null or a list containing no null elements.
70-71: LGTM!Correct use of JSpecify type-use annotation syntax (
CommonAttribute.@Nullable UseContent) for the return type of a nested class.src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/EnumConverter.java (1)
62-72: LGTM!The
@Nullableannotations correctly document the existing null-handling behavior: null input yields null output. This is appropriate for a converter that may receive optional values.src/main/java/com/github/_1c_syntax/bsl/reader/common/converter/ValueTypeConverter.java (1)
120-123: LGTM! Appropriate nullability annotation.The
@Nullableannotation correctly documents thatgetTypemay return null (sinceMap.get()can return null), and all call sites properly check for null before use.src/main/java/com/github/_1c_syntax/bsl/mdo/storage/package-info.java (1)
32-35: LGTM! Package-level nullability migration.The migration from legacy JSR-305 annotations to JSpecify's
@NullMarkedis consistent with the project-wide null-safety strategy. Based on learnings, package-level annotations for static analysis are the accepted approach in this project.src/main/java/com/github/_1c_syntax/bsl/mdo/children/package-info.java (1)
26-29: LGTM! Consistent nullability migration.The package-level
@NullMarkedannotation aligns with the project-wide migration to JSpecify nullability standards.src/main/java/com/github/_1c_syntax/bsl/mdo/storage/QuerySource.java (1)
31-36: LGTM! Nullability contract preserved.With the package now marked as
@NullMarked, thetextQueryparameter remains non-null by default. The removal of explicit@NonNullannotations relies on the package-level contract, which is the accepted pattern for this migration.src/main/java/com/github/_1c_syntax/bsl/reader/common/context/FormElementReaderContext.java (1)
58-76: LGTM! Correct nullability annotations.The
@Nullableannotations onelementTypefield andfieldTypereturn type accurately reflect the nullability contract, asfieldTypecan return null fromMap.get()or the superclass method.src/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDReaderContext.java (2)
128-134: LGTM! Appropriate nullability for setValue parameter.The
@Nullableannotation on thevalueparameter correctly reflects that the method can accept null values, which is appropriate given the polymorphic nature of metadata value handling.
87-116: No behavioral issues with the constructor initialization changes.The two identified changes are both valid:
Lines 92-94 null check: The
getRealClass()method returns@Nullable, and adding an explicit check withIllegalArgumentExceptionis a valid defensive improvement. This pattern is already used elsewhere in the codebase (e.g.,MetaDataObjectConverterthrowsIllegalStateExceptionfor the same condition).Lines 112-113 setValue routing: While the calls now go through the overridden
setValue()method instead of directly tosuper.setValue(), the behavior is functionally equivalent. Sinceuuid(String) andsupportVariant(SupportVariant enum) are neverMDReaderContextinstances, they will immediately delegate tosuper.setValue()regardless of the routing.src/main/java/com/github/_1c_syntax/bsl/reader/common/context/MDCReaderContext.java (3)
73-91: Verify the setValue routing change in constructor.Lines 87-88: Similar to
MDReaderContext, the change fromsuper.setValue(...)tosetValue(...)routes through the overridden method. While functionally equivalent for non-MDOType values, this represents a behavioral change in the call path that should be verified with tests.
59-60: LGTM! Correct nullable field annotation.The
@Nullableannotation oncompatibilityModeis appropriate, as the code explicitly checks for null at Line 107 (if (compatibilityMode == null)).
94-100: LGTM! Appropriate nullability for polymorphic value handling.The
@Nullableannotation on thevalueparameter correctly reflects the flexible nature of metadata value handling in this context builder.src/main/java/com/github/_1c_syntax/bsl/mdo/storage/DataCompositionSchema.java (2)
58-80: LGTM! Consistent null-safety under package-level @NullMarked.With the package annotated as
@NullMarked, the constructor parameters and record fields remain non-null by default. TheObjects.requireNonNull(dataSets)at Line 70 serves as defensive runtime validation, which is consistent with the compile-time non-null contract established by the package annotation.
91-104: LGTM! Record fields inherit package-level non-null contract.The removal of explicit
@NonNullannotations from theDataSetandDataSetFieldrecords is appropriate, as the package-level@NullMarkedannotation provides the non-null default for all fields.src/main/java/com/github/_1c_syntax/bsl/reader/common/context/AbstractReaderContext.java (5)
38-38: LGTM! JSpecify import added for null-safety migration.The import is correctly added to support the @nullable annotations used throughout this file as part of the null-safety modernization.
115-118: LGTM! Correct @nullable annotation on optional field.The
lastValuefield is appropriately marked as @nullable since it represents the most recently read value and may not always be present.
126-138: LGTM! Constructor parameters now rely on package-level non-null defaults.The removal of explicit
@NonNullannotations aligns with the migration to JSpecify's@NullMarkedpackage-level annotation, which makes all parameters non-null by default. Static analysis will enforce these constraints at call sites.Based on learnings, this project uses package-level annotations for null-safety through static analysis, which is the approach being followed here.
146-168: LGTM! Proper handling of @nullable parameter.The
valueparameter is correctly annotated as @nullable, and the implementation properly guards against null with an explicit check at line 147. This allows the method to gracefully handle optional values during object construction.
174-179: LGTM! @nullable return type correctly documents optional result.The @nullable annotation and updated comment accurately reflect that this method returns null when the field type cannot be determined. This is appropriate behavior for a field type lookup method.
|
сгенерируй недостающие тесты, чтобы sonarqube не ругался |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 6136 🏁 Script executed: Length of output: 3559 🏁 Script executed: Length of output: 5425 @theshadowco, я проанализировал PR и выявил отсутствие тестов для новых констант EMPTY и классов storage. Сгенерирую недостающие тесты: 1. Тест для QuerySource: /*
* This file is a part of MDClasses.
*
* Copyright (c) 2019 - 2025
* Tymko Oleg <[email protected]>, Maximov Valery <[email protected]> and contributors
*
* SPDX-License-Identifier: LGPL-3.0-or-later
*
* MDClasses is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
*
* MDClasses is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with MDClasses.
*/
package com.github._1c_syntax.bsl.mdo.storage;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThat;
class QuerySourceTest {
@Test
void testEmptyConstant() {
assertThat(QuerySource.EMPTY).isNotNull();
assertThat(QuerySource.EMPTY.line()).isZero();
assertThat(QuerySource.EMPTY.column()).isZero();
assertThat(QuerySource.EMPTY.textQuery()).isEmpty();
}
@Test
void testRecordCreation() {
var querySource = new QuerySource(10, 5, "SELECT * FROM Table");
assertThat(querySource.line()).isEqualTo(10);
assertThat(querySource.column()).isEqualTo(5);
assertThat(querySource.textQuery()).isEqualTo("SELECT * FROM Table");
}
@Test
void testRecordWithEmptyQuery() {
var querySource = new QuerySource(0, 0, "");
assertThat(querySource).isNotNull();
assertThat(querySource.textQuery()).isEmpty();
}
@Test
void testRecordEquality() {
var querySource1 = new QuerySource(1, 2, "query");
var querySource2 = new QuerySource(1, 2, "query");
assertThat(querySource1).isEqualTo(querySource2);
}
}2. Тест для Handler: /*
* This file is a part of MDClasses.
*
* Copyright (c) 2019 - 2025
* Tymko Oleg <[email protected]>, Maximov Valery <[email protected]> and contributors
*
* SPDX-License-Identifier: LGPL-3.0-or-later
*
* MDClasses is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
*
* MDClasses is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with MDClasses.
*/
package com.github._1c_syntax.bsl.mdo.support;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThat;
class HandlerTest {
@Test
void testEmptyConstant() {
assertThat(Handler.EMPTY).isNotNull();
assertThat(Handler.EMPTY.isEmpty()).isTrue();
assertThat(Handler.EMPTY.getMethodPath()).isEmpty();
assertThat(Handler.EMPTY.getModuleName()).isEmpty();
assertThat(Handler.EMPTY.getMethodName()).isEmpty();
}
@Test
void testNullPath() {
var handler = new Handler(null);
assertThat(handler.isEmpty()).isTrue();
assertThat(handler.getMethodPath()).isEmpty();
assertThat(handler.getModuleName()).isEmpty();
assertThat(handler.getMethodName()).isEmpty();
}
@Test
void testEmptyPath() {
var handler = new Handler("");
assertThat(handler.isEmpty()).isTrue();
assertThat(handler.getMethodPath()).isEmpty();
}
@Test
void testValidHandler() {
var handler = new Handler("Configuration.CommonModule.MethodName");
assertThat(handler.isEmpty()).isFalse();
assertThat(handler.getMethodPath()).isEqualTo("Configuration.CommonModule.MethodName");
assertThat(handler.getModuleName()).isEqualTo("CommonModule");
assertThat(handler.getMethodName()).isEqualTo("MethodName");
}
@Test
void testHandlerWithModuleOnly() {
var handler = new Handler("Configuration.CommonModule");
assertThat(handler.isEmpty()).isFalse();
assertThat(handler.getModuleName()).isEqualTo("CommonModule");
assertThat(handler.getMethodName()).isEmpty();
}
@Test
void testHandlerWithShortPath() {
var handler = new Handler("Module");
assertThat(handler.isEmpty()).isFalse();
assertThat(handler.getModuleName()).isEmpty();
assertThat(handler.getMethodName()).isEmpty();
}
}3. Тест для RoleData (дополнение существующего): /*
* This file is a part of MDClasses.
*
* Copyright (c) 2019 - 2025
* Tymko Oleg <[email protected]>, Maximov Valery <[email protected]> and contributors
*
* SPDX-License-Identifier: LGPL-3.0-or-later
*
* MDClasses is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
*
* MDClasses is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with MDClasses.
*/
package com.github._1c_syntax.bsl.mdo.storage;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThat;
class RoleDataTest {
@Test
void testEmptyConstant() {
assertThat(RoleData.EMPTY).isNotNull();
assertThat(RoleData.EMPTY.getObjectRights()).isEmpty();
}
@Test
void testBuilderWithEmptyList() {
var roleData = RoleData.builder().build();
assertThat(roleData).isNotNull();
assertThat(roleData.getObjectRights()).isEmpty();
}
@Test
void testObjectRight() {
var objectRight = new RoleData.ObjectRight("TestObject", null);
assertThat(objectRight).isNotNull();
assertThat(objectRight.name()).isEqualTo("TestObject");
assertThat(objectRight.rights()).isNull();
}
@Test
void testRight() {
var right = new RoleData.Right("Read", true);
assertThat(right).isNotNull();
assertThat(right.name()).isEqualTo("Read");
assertThat(right.value()).isTrue();
}
}4. Тест для XdtoPackageData (дополнение): /*
* This file is a part of MDClasses.
*
* Copyright (c) 2019 - 2025
* Tymko Oleg <[email protected]>, Maximov Valery <[email protected]> and contributors
*
* SPDX-License-Identifier: LGPL-3.0-or-later
*
* MDClasses is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
*
* MDClasses is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with MDClasses.
*/
package com.github._1c_syntax.bsl.mdo.storage;
import org.junit.jupiter.api.Test;
import static org.assertj.core.api.Assertions.assertThat;
class XdtoPackageDataTest {
@Test
void testEmptyConstant() {
assertThat(XdtoPackageData.EMPTY).isNotNull();
assertThat(XdtoPackageData.EMPTY.targetNamespace()).isEmpty();
assertThat(XdtoPackageData.EMPTY.imports()).isEmpty();
assertThat(XdtoPackageData.EMPTY.valueTypes()).isEmpty();
assertThat(XdtoPackageData.EMPTY.objectTypes()).isEmpty();
assertThat(XdtoPackageData.EMPTY.properties()).isEmpty();
}
@Test
void testValueType() {
var valueType = new XdtoPackageData.ValueType(
"TestType",
"BaseType",
"Atomic",
java.util.List.of("Value1", "Value2")
);
assertThat(valueType).isNotNull();
assertThat(valueType.name()).isEqualTo("TestType");
assertThat(valueType.base()).isEqualTo("BaseType");
assertThat(valueType.variety()).isEqualTo("Atomic");
assertThat(valueType.enumerations()).hasSize(2);
}
@Test
void testObjectType() {
var objectType = new XdtoPackageData.ObjectType(
"TestObject",
"BaseObject",
java.util.List.of()
);
assertThat(objectType).isNotNull();
assertThat(objectType.name()).isEqualTo("TestObject");
assertThat(objectType.base()).isEqualTo("BaseObject");
assertThat(objectType.properties()).isEmpty();
}
@Test
void testProperty() {
var property = new XdtoPackageData.Property(
"TestProperty",
"string",
0,
1,
false,
"Element",
java.util.List.of()
);
assertThat(property).isNotNull();
assertThat(property.name()).isEqualTo("TestProperty");
assertThat(property.type()).isEqualTo("string");
assertThat(property.lowerBound()).isZero();
assertThat(property.upperBound()).isOne();
assertThat(property.nillable()).isFalse();
assertThat(property.form()).isEqualTo("Element");
assertThat(property.typeDef()).isEmpty();
}
}Эти тесты покрывают:
Создайте эти файлы в соответствующих директориях:
🧠 Learnings used |
|
сгенерируй ендостающие тесты для классов |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 13874 🏁 Script executed: Length of output: 7485 @theshadowco, я проанализировал классы 1. Тест для MDOReader: /*
* This file is a part of MDClasses.
*
* Copyright (c) 2019 - 2025
* Tymko Oleg <[email protected]>, Maximov Valery <[email protected]> and contributors
*
* SPDX-License-Identifier: LGPL-3.0-or-later
*
* MDClasses is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
*
* MDClasses is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with MDClasses.
*/
package com.github._1c_syntax.bsl.reader;
import com.github._1c_syntax.bsl.mdclasses.MDCReadSettings;
import com.github._1c_syntax.bsl.mdo.Configuration;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import java.nio.file.Path;
import java.nio.file.Paths;
import static org.assertj.core.api.Assertions.assertThat;
class MDOReaderTest {
@Test
void testReadConfigurationWithValidPath() {
var path = Paths.get("src/test/resources/ext/edt/ssl_3_1");
var configuration = MDOReader.readConfiguration(path);
assertThat(configuration).isNotNull();
assertThat(configuration).isInstanceOf(Configuration.class);
}
@Test
void testReadConfigurationWithSettings() {
var path = Paths.get("src/test/resources/ext/edt/ssl_3_1");
var settings = MDCReadSettings.DEFAULT;
var configuration = MDOReader.readConfiguration(path, settings);
assertThat(configuration).isNotNull();
assertThat(configuration).isInstanceOf(Configuration.class);
}
@Test
void testReadConfigurationWithNullPath() {
var configuration = MDOReader.readConfiguration(null);
assertThat(configuration).isNotNull();
assertThat(configuration).isEqualTo(Configuration.EMPTY);
}
@Test
void testReadConfigurationWithNullPathAndSettings() {
var settings = MDCReadSettings.DEFAULT;
var configuration = MDOReader.readConfiguration(null, settings);
assertThat(configuration).isNotNull();
assertThat(configuration).isEqualTo(Configuration.EMPTY);
}
@Test
void testReadConfigurationWithNonExistentPath() {
var path = Paths.get("src/test/resources/nonexistent");
var configuration = MDOReader.readConfiguration(path);
assertThat(configuration).isNotNull();
assertThat(configuration).isEqualTo(Configuration.EMPTY);
}
@ParameterizedTest
@ValueSource(strings = {
"src/test/resources/ext/edt/ssl_3_1",
"src/test/resources/ext/designer/ssl_3_1/src/cf"
})
void testReadConfigurationDifferentSources(String pathString) {
var path = Paths.get(pathString);
var configuration = MDOReader.readConfiguration(path);
assertThat(configuration).isNotNull();
assertThat(configuration).isInstanceOf(Configuration.class);
}
@Test
void testReadWithNullFolder() {
var result = MDOReader.read(null, "CommonModules.Test");
assertThat(result).isNull();
}
@Test
void testReadWithNullFullName() {
var path = Paths.get("src/test/resources/ext/edt/ssl_3_1");
var result = MDOReader.read(path, null);
// Should handle null gracefully
assertThat(result).isNull();
}
@Test
void testReadWithValidParameters() {
var path = Paths.get("src/test/resources/ext/edt/ssl_3_1");
var result = MDOReader.read(path, "CommonModules.АвтономнаяРабота");
// May return null if not found, but should not throw
assertThat(result).isNotNull();
}
@Test
void testReadWithSettings() {
var path = Paths.get("src/test/resources/ext/edt/ssl_3_1");
var settings = MDCReadSettings.DEFAULT;
var result = MDOReader.read(path, "CommonModules.АвтономнаяРабота", settings);
assertThat(result).isNotNull();
}
@Test
void testReadWithNonExistentObject() {
var path = Paths.get("src/test/resources/ext/edt/ssl_3_1");
var result = MDOReader.read(path, "CommonModules.NonExistent");
// Should return null for non-existent object
assertThat(result).isNull();
}
@Test
void testReadExternalSource() {
var path = Paths.get("src/test/resources/ext/edt/external/ExternalDataProcessors/ТестоваяОбработка/ТестоваяОбработка.mdo");
var externalSource = MDOReader.readExternalSource(path);
assertThat(externalSource).isNotNull();
}
@Test
void testReadExternalSourceWithSettings() {
var path = Paths.get("src/test/resources/ext/edt/external/ExternalDataProcessors/ТестоваяОбработка/ТестоваяОбработка.mdo");
var settings = MDCReadSettings.SKIP_SUPPORT;
var externalSource = MDOReader.readExternalSource(path, settings);
assertThat(externalSource).isNotNull();
}
@Test
void testReadExternalSourceWithNullPath() {
var externalSource = MDOReader.readExternalSource(null);
assertThat(externalSource).isNotNull();
assertThat(externalSource).isEqualTo(Configuration.EMPTY);
}
@Test
void testReadExternalSourceWithInvalidPath() {
var path = Paths.get("src/test/resources/nonexistent.mdo");
var externalSource = MDOReader.readExternalSource(path);
assertThat(externalSource).isNotNull();
assertThat(externalSource).isEqualTo(Configuration.EMPTY);
}
@Test
void testReadConfigurationFromFile() {
var path = Paths.get("src/test/resources/ext/edt/ssl_3_1/configuration/Configuration.mdo");
var configuration = MDOReader.readConfiguration(path);
assertThat(configuration).isNotNull();
assertThat(configuration).isInstanceOf(Configuration.class);
}
@Test
void testReadConfigurationFromDesignerFile() {
var path = Paths.get("src/test/resources/ext/designer/ssl_3_1/src/Configuration/Configuration.xml");
var configuration = MDOReader.readConfiguration(path);
assertThat(configuration).isNotNull();
assertThat(configuration).isInstanceOf(Configuration.class);
}
@Test
void testReadWithFileParameter() {
var path = Paths.get("src/test/resources/ext/edt/ssl_3_1/configuration/Configuration.mdo");
var result = MDOReader.read(path, "Configuration");
assertThat(result).isNotNull();
}
@Test
void testReadConfigurationWithEmptyPath() {
var path = Paths.get("");
var configuration = MDOReader.readConfiguration(path);
assertThat(configuration).isNotNull();
}
}2. Тест для TransformationUtils: /*
* This file is a part of MDClasses.
*
* Copyright (c) 2019 - 2025
* Tymko Oleg <[email protected]>, Maximov Valery <[email protected]> and contributors
*
* SPDX-License-Identifier: LGPL-3.0-or-later
*
* MDClasses is free software; you can redistribute it and/or
* modify it under the terms of the GNU Lesser General Public
* License as published by the Free Software Foundation; either
* version 3.0 of the License, or (at your option) any later version.
*
* MDClasses is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with MDClasses.
*/
package com.github._1c_syntax.bsl.reader.common;
import lombok.Builder;
import lombok.Data;
import lombok.Singular;
import lombok.Value;
import org.junit.jupiter.api.Test;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.List;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
class TransformationUtilsTest {
@Test
void testSetValueWithValidParameters() {
var builder = TestClass.builder();
TransformationUtils.setValue(builder, "name", "TestName");
var result = builder.build();
assertThat(result.getName()).isEqualTo("TestName");
}
@Test
void testSetValueWithNullValue() {
var builder = TestClass.builder();
TransformationUtils.setValue(builder, "name", null);
var result = builder.build();
// Null values should not be set
assertThat(result.getName()).isNull();
}
@Test
void testSetValueWithNullMethodName() {
var builder = TestClass.builder();
// Should handle null method name gracefully
TransformationUtils.setValue(builder, null, "value");
var result = builder.build();
assertThat(result).isNotNull();
}
@Test
void testSetValueWithListParameter() {
var builder = TestClassWithList.builder();
TransformationUtils.setValue(builder, "items", List.of("item1", "item2"));
var result = builder.build();
assertThat(result.getItems()).containsExactly("item1", "item2");
}
@Test
void testSetValueWithSingularParameter() {
var builder = TestClassWithList.builder();
TransformationUtils.setValue(builder, "item", "singleItem");
var result = builder.build();
assertThat(result.getItems()).contains("singleItem");
}
@Test
void testInvokeWithValidMethod() {
var mutableObject = new MutableTestClass();
TransformationUtils.invoke(mutableObject, "reset");
assertThat(mutableObject.isReset()).isTrue();
}
@Test
void testInvokeWithNullMethodName() {
var mutableObject = new MutableTestClass();
// Should handle null gracefully
TransformationUtils.invoke(mutableObject, null);
assertThat(mutableObject.isReset()).isFalse();
}
@Test
void testInvokeWithNonExistentMethod() {
var mutableObject = new MutableTestClass();
// Should not throw, just log error
TransformationUtils.invoke(mutableObject, "nonExistentMethod");
assertThat(mutableObject.isReset()).isFalse();
}
@Test
void testFieldType() {
var builder = TestClass.builder();
var type = TransformationUtils.fieldType(builder, "name");
assertThat(type).isNotNull();
assertThat(type.getTypeName()).contains("String");
}
@Test
void testFieldTypeWithNullMethodName() {
var builder = TestClass.builder();
var type = TransformationUtils.fieldType(builder, null);
assertThat(type).isNull();
}
@Test
void testFieldTypeWithNonExistentField() {
var builder = TestClass.builder();
var type = TransformationUtils.fieldType(builder, "nonExistent");
assertThat(type).isNull();
}
@Test
void testBuilder() {
var builder = TransformationUtils.builder(TestClass.class);
assertThat(builder).isNotNull();
assertThat(builder).isInstanceOf(TestClass.TestClassBuilder.class);
}
@Test
void testBuilderWithClassWithoutBuilder() {
assertThatThrownBy(() -> TransformationUtils.builder(String.class))
.isInstanceOf(IllegalArgumentException.class)
.hasMessageContaining("Incorrect class");
}
@Test
void testToBuilder() {
var original = TestClass.builder().name("Original").value(42).build();
var builder = TransformationUtils.toBuilder(original);
assertThat(builder).isNotNull();
assertThat(builder).isInstanceOf(TestClass.TestClassBuilder.class);
}
@Test
void testToBuilderWithClassWithoutToBuilder() {
var simpleObject = new SimpleTestClass("test");
var builder = TransformationUtils.toBuilder(simpleObject);
assertThat(builder).isNull();
}
@Test
void testBuildWithBuilder() {
var builder = TestClass.builder().name("Test").value(100);
var result = TransformationUtils.build(builder);
assertThat(result).isNotNull();
assertThat(result).isInstanceOf(TestClass.class);
assertThat(((TestClass) result).getName()).isEqualTo("Test");
assertThat(((TestClass) result).getValue()).isEqualTo(100);
}
@Test
void testBuildWithBuilderAndPath() {
var builder = TestClass.builder().name("Test").value(100);
var path = Paths.get("test/path");
var result = TransformationUtils.build(builder, path);
assertThat(result).isNotNull();
assertThat(result).isInstanceOf(TestClass.class);
}
@Test
void testBuildWithNullBuilder() {
var result = TransformationUtils.build(null);
assertThat(result).isNull();
}
@Test
void testBuildWithInvalidBuilder() {
var result = TransformationUtils.build("not a builder");
assertThat(result).isNull();
}
@Test
void testSetValueWithInvalidMethodName() {
var builder = TestClass.builder();
// Should not throw, just log error
TransformationUtils.setValue(builder, "nonExistentMethod", "value");
var result = builder.build();
assertThat(result).isNotNull();
}
@Test
void testFieldTypeWithListParameter() {
var builder = TestClassWithList.builder();
var type = TransformationUtils.fieldType(builder, "items");
assertThat(type).isNotNull();
}
// Test helper classes
@Data
@Builder
private static class TestClass {
private String name;
private int value;
}
@Data
@Builder
private static class TestClassWithList {
@Singular
private List<String> items;
}
@Value
private static class SimpleTestClass {
String name;
}
private static class MutableTestClass {
private boolean reset = false;
public void reset() {
this.reset = true;
}
public boolean isReset() {
return reset;
}
}
}Эти тесты покрывают: Для MDOReader:
Для TransformationUtils:
Создайте эти файлы:
🧠 Learnings used |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
src/test/java/com/github/_1c_syntax/bsl/mdo/storage/QuerySourceTest.java (1)
47-53: Consider removing redundant test.The
testRecordWithEmptyQuerymethod tests the same scenario as theEMPTYconstant (line 0, column 0, empty string). SincetestEmptyConstantalready covers this case, this test adds minimal value.src/test/java/com/github/_1c_syntax/bsl/mdo/storage/XdtoPackageDataTest.java (1)
30-38: Good baseline test for the EMPTY constant.The test verifies that
EMPTYexists and all collections are empty, which provides basic coverage for this sentinel value.Optional: Consider additional assertions for EMPTY constant behavior
You could optionally verify that
EMPTYreturns the same instance (identity) or test immutability guarantees:@Test void testEmptyConstant() { assertThat(XdtoPackageData.EMPTY).isNotNull(); assertThat(XdtoPackageData.EMPTY.targetNamespace()).isEmpty(); assertThat(XdtoPackageData.EMPTY.imports()).isEmpty(); assertThat(XdtoPackageData.EMPTY.valueTypes()).isEmpty(); assertThat(XdtoPackageData.EMPTY.objectTypes()).isEmpty(); assertThat(XdtoPackageData.EMPTY.properties()).isEmpty(); // Optional: verify immutability of collections assertThat(XdtoPackageData.EMPTY.imports()).isUnmodifiable(); }src/test/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtilsTest.java (1)
135-144: Consider strengthening the path parameter assertion.The test verifies that the result is not null and of the correct type, but it doesn't verify that the
pathparameter is actually used or has any effect. If the path parameter influences the built object, consider adding assertions to verify this behavior.src/test/java/com/github/_1c_syntax/bsl/reader/MDOReaderTest.java (1)
91-106: Consider more specific assertions for external source tests.The tests for
readExternalSourcemethods only assert non-null. Consider adding.isInstanceOf(ExternalReport.class)or verifying specific properties to make the tests more meaningful and catch potential type issues.🔎 Proposed enhancement
@Test void testReadExternalSource() { var path = Paths.get("src/test/resources/ext/edt/external/ExternalDataProcessors/ТестоваяОбработка/ТестоваяОбработка.mdo"); var externalSource = MDOReader.readExternalSource(path); - assertThat(externalSource).isNotNull(); + assertThat(externalSource) + .isNotNull() + .isInstanceOf(ExternalReport.class); } @Test void testReadExternalSourceWithSettings() { var path = Paths.get("src/test/resources/ext/edt/external/ExternalDataProcessors/ТестоваяОбработка/ТестоваяОбработка.mdo"); var settings = MDCReadSettings.SKIP_SUPPORT; var externalSource = MDOReader.readExternalSource(path, settings); - assertThat(externalSource).isNotNull(); + assertThat(externalSource) + .isNotNull() + .isInstanceOf(ExternalReport.class); }src/test/java/com/github/_1c_syntax/bsl/mdo/support/HandlerTest.java (2)
49-55: Consider adding assertions for all getters for consistency.The test validates
isEmpty()andgetMethodPath()but omitsgetModuleName()andgetMethodName(), unliketestNullPathwhich checks all getters. For test completeness and consistency, consider verifying all relevant getters.🔎 Suggested addition
@Test void testEmptyPath() { var handler = new Handler(""); assertThat(handler.isEmpty()).isTrue(); assertThat(handler.getMethodPath()).isEmpty(); + assertThat(handler.getModuleName()).isEmpty(); + assertThat(handler.getMethodName()).isEmpty(); }
67-74: Consider verifyinggetMethodPath()for consistency.The test checks module and method names but omits
getMethodPath(). For completeness, consider asserting the expected value ofgetMethodPath().🔎 Suggested addition
@Test void testHandlerWithModuleOnly() { var handler = new Handler("Configuration.CommonModule"); assertThat(handler.isEmpty()).isFalse(); + assertThat(handler.getMethodPath()).isEqualTo("Configuration.CommonModule"); assertThat(handler.getModuleName()).isEqualTo("CommonModule"); assertThat(handler.getMethodName()).isEmpty(); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/main/java/com/github/_1c_syntax/bsl/reader/common/context/AbstractReaderContext.javasrc/test/java/com/github/_1c_syntax/bsl/mdo/storage/QuerySourceTest.javasrc/test/java/com/github/_1c_syntax/bsl/mdo/storage/RoleDataTest.javasrc/test/java/com/github/_1c_syntax/bsl/mdo/storage/XdtoPackageDataTest.javasrc/test/java/com/github/_1c_syntax/bsl/mdo/support/HandlerTest.javasrc/test/java/com/github/_1c_syntax/bsl/reader/MDOReaderTest.javasrc/test/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtilsTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/github/_1c_syntax/bsl/reader/common/context/AbstractReaderContext.java
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: theshadowco
Repo: 1c-syntax/mdclasses PR: 524
File: src/main/java/com/github/_1c_syntax/bsl/mdo/support/UsePurposes.java:61-69
Timestamp: 2025-10-06T14:38:27.354Z
Learning: В проекте mdclasses используются JSR-305 аннотации (ParametersAreNonnullByDefault) на уровне пакетов для обеспечения null-safety через статический анализ, что является приемлемым подходом вместо явных runtime проверок.
📚 Learning: 2025-09-05T10:03:02.229Z
Learnt from: johnnyshut
Repo: 1c-syntax/mdclasses PR: 515
File: src/test/java/com/github/_1c_syntax/bsl/mdo/AccountingRegisterTest.java:35-37
Timestamp: 2025-09-05T10:03:02.229Z
Learning: The MDTestUtils.getMDWithSimpleTest method already correctly handles optional 4th parameter "_edt" through ArgumentsAccessor - it doesn't need modification to process the additional CSV parameter for subdirectory selection.
Applied to files:
src/test/java/com/github/_1c_syntax/bsl/mdo/storage/XdtoPackageDataTest.java
🧬 Code graph analysis (1)
src/test/java/com/github/_1c_syntax/bsl/reader/MDOReaderTest.java (1)
src/test/java/com/github/_1c_syntax/bsl/test_utils/assertions/Assertions.java (1)
Assertions(24-29)
⏰ 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). (18)
- GitHub Check: QA
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
🔇 Additional comments (10)
src/test/java/com/github/_1c_syntax/bsl/mdo/storage/RoleDataTest.java (3)
34-38: LGTM!The test correctly validates the EMPTY sentinel constant and its expected behavior.
40-46: LGTM!The test correctly validates the builder pattern with default empty state.
57-64: LGTM!The test correctly validates the Right inner class construction and property access.
src/test/java/com/github/_1c_syntax/bsl/mdo/storage/XdtoPackageDataTest.java (1)
40-90: The existing tests provide adequate coverage for the record types.The tests correctly cover basic record construction and field access. The suggested null-safety enhancements are not applicable here because all fields in
XdtoPackageDataand its nested records (ValueType,ObjectType,Property) are non-nullable by default. The package uses the@NullMarkedannotation from JSpecify, which declares that all fields are non-null unless explicitly marked with@Nullable. No fields in these records carry@Nullableannotations, so the existing tests that construct valid instances with non-null values are appropriate.Likely an incorrect or invalid review comment.
src/test/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtilsTest.java (2)
1-36: LGTM!The license header and imports are appropriate. The test uses modern frameworks (JUnit 5, AssertJ) and includes necessary Lombok annotations for the test helper classes.
171-196: LGTM!The test helper classes are well-structured and serve clear purposes:
TestClassandTestClassWithListprovide builder-based test subjects with different field typesSimpleTestClass(record) enables testing of classes without builder supportMutableTestClassprovides mutable state for method invocation testingsrc/test/java/com/github/_1c_syntax/bsl/reader/MDOReaderTest.java (3)
1-34: LGTM!The license header and imports are properly structured and include all necessary dependencies for the test class.
37-80: Good coverage of configuration reading scenarios.The tests appropriately cover valid paths, settings variations, non-existent paths (returning EMPTY), and different source formats (EDT/Designer). The use of parameterized tests for different sources is efficient.
82-89: Verify the null return is intentional for non-existent objects.This test expects
nullwhen reading a non-existent object, which may be inconsistent with the pattern established elsewhere (e.g.,readConfigurationreturningConfiguration.EMPTYfor non-existent paths). Confirm whetherMDOReader.read()should follow the same EMPTY pattern asreadConfiguration()for consistency with this null-safety PR.src/test/java/com/github/_1c_syntax/bsl/mdo/support/HandlerTest.java (1)
1-27: LGTM!License header, package declaration, and imports are all correct.
| class QuerySourceTest { | ||
|
|
||
| @Test | ||
| void testEmptyConstant() { | ||
| assertThat(QuerySource.EMPTY).isNotNull(); | ||
| assertThat(QuerySource.EMPTY.line()).isZero(); | ||
| assertThat(QuerySource.EMPTY.column()).isZero(); | ||
| assertThat(QuerySource.EMPTY.textQuery()).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void testRecordCreation() { | ||
| var querySource = new QuerySource(10, 5, "SELECT * FROM Table"); | ||
|
|
||
| assertThat(querySource.line()).isEqualTo(10); | ||
| assertThat(querySource.column()).isEqualTo(5); | ||
| assertThat(querySource.textQuery()).isEqualTo("SELECT * FROM Table"); | ||
| } | ||
|
|
||
| @Test | ||
| void testRecordWithEmptyQuery() { | ||
| var querySource = new QuerySource(0, 0, ""); | ||
|
|
||
| assertThat(querySource).isNotNull(); | ||
| assertThat(querySource.textQuery()).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void testRecordEquality() { | ||
| var querySource1 = new QuerySource(1, 2, "query"); | ||
| var querySource2 = new QuerySource(1, 2, "query"); | ||
|
|
||
| assertThat(querySource1).isEqualTo(querySource2); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add test for null textQuery parameter.
Given that this PR removes @NonNull from the textQuery parameter to support null-safety, a test should verify the behavior when null is passed to the constructor. This documents whether null is permitted and ensures the null-safety contract is clear.
🔎 Proposed test for null textQuery
+ @Test
+ void testRecordWithNullQuery() {
+ var querySource = new QuerySource(10, 5, null);
+
+ assertThat(querySource).isNotNull();
+ assertThat(querySource.line()).isEqualTo(10);
+ assertThat(querySource.column()).isEqualTo(5);
+ assertThat(querySource.textQuery()).isNull();
+ }📝 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.
| class QuerySourceTest { | |
| @Test | |
| void testEmptyConstant() { | |
| assertThat(QuerySource.EMPTY).isNotNull(); | |
| assertThat(QuerySource.EMPTY.line()).isZero(); | |
| assertThat(QuerySource.EMPTY.column()).isZero(); | |
| assertThat(QuerySource.EMPTY.textQuery()).isEmpty(); | |
| } | |
| @Test | |
| void testRecordCreation() { | |
| var querySource = new QuerySource(10, 5, "SELECT * FROM Table"); | |
| assertThat(querySource.line()).isEqualTo(10); | |
| assertThat(querySource.column()).isEqualTo(5); | |
| assertThat(querySource.textQuery()).isEqualTo("SELECT * FROM Table"); | |
| } | |
| @Test | |
| void testRecordWithEmptyQuery() { | |
| var querySource = new QuerySource(0, 0, ""); | |
| assertThat(querySource).isNotNull(); | |
| assertThat(querySource.textQuery()).isEmpty(); | |
| } | |
| @Test | |
| void testRecordEquality() { | |
| var querySource1 = new QuerySource(1, 2, "query"); | |
| var querySource2 = new QuerySource(1, 2, "query"); | |
| assertThat(querySource1).isEqualTo(querySource2); | |
| } | |
| class QuerySourceTest { | |
| @Test | |
| void testEmptyConstant() { | |
| assertThat(QuerySource.EMPTY).isNotNull(); | |
| assertThat(QuerySource.EMPTY.line()).isZero(); | |
| assertThat(QuerySource.EMPTY.column()).isZero(); | |
| assertThat(QuerySource.EMPTY.textQuery()).isEmpty(); | |
| } | |
| @Test | |
| void testRecordCreation() { | |
| var querySource = new QuerySource(10, 5, "SELECT * FROM Table"); | |
| assertThat(querySource.line()).isEqualTo(10); | |
| assertThat(querySource.column()).isEqualTo(5); | |
| assertThat(querySource.textQuery()).isEqualTo("SELECT * FROM Table"); | |
| } | |
| @Test | |
| void testRecordWithEmptyQuery() { | |
| var querySource = new QuerySource(0, 0, ""); | |
| assertThat(querySource).isNotNull(); | |
| assertThat(querySource.textQuery()).isEmpty(); | |
| } | |
| @Test | |
| void testRecordEquality() { | |
| var querySource1 = new QuerySource(1, 2, "query"); | |
| var querySource2 = new QuerySource(1, 2, "query"); | |
| assertThat(querySource1).isEqualTo(querySource2); | |
| } | |
| @Test | |
| void testRecordWithNullQuery() { | |
| var querySource = new QuerySource(10, 5, null); | |
| assertThat(querySource).isNotNull(); | |
| assertThat(querySource.line()).isEqualTo(10); | |
| assertThat(querySource.column()).isEqualTo(5); | |
| assertThat(querySource.textQuery()).isNull(); | |
| } | |
| } |
🤖 Prompt for AI Agents
In src/test/java/com/github/_1c_syntax/bsl/mdo/storage/QuerySourceTest.java
around lines 28 to 61, add a unit test that constructs a QuerySource with a null
textQuery (e.g., new QuerySource(0, 0, null)) and assert that construction does
not throw and that querySource.textQuery() returns null to document and verify
the new null-allowed behavior; implement the test as a simple @Test method
alongside existing tests, using the same assertion library to check non-throwing
construction and that textQuery() is null.
| @Test | ||
| void testObjectRight() { | ||
| var objectRight = new RoleData.ObjectRight(MdoReference.create("Catalogs.TestObject"), Collections.emptyList()); | ||
|
|
||
| assertThat(objectRight).isNotNull(); | ||
| assertThat(objectRight.name().getMdoRef()).isEqualTo("Catalog.TestObject"); | ||
| assertThat(objectRight.rights()).isEmpty(); | ||
| } |
There was a problem hiding this comment.
Fix the string mismatch that will cause test failure.
Line 50 creates an MdoReference with "Catalogs.TestObject" (plural), but line 53 asserts the result should be "Catalog.TestObject" (singular). This mismatch will cause the test to fail.
🔎 Proposed fix
- var objectRight = new RoleData.ObjectRight(MdoReference.create("Catalogs.TestObject"), Collections.emptyList());
+ var objectRight = new RoleData.ObjectRight(MdoReference.create("Catalog.TestObject"), Collections.emptyList());
assertThat(objectRight).isNotNull();
assertThat(objectRight.name().getMdoRef()).isEqualTo("Catalog.TestObject");
assertThat(objectRight.rights()).isEmpty();📝 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.
| @Test | |
| void testObjectRight() { | |
| var objectRight = new RoleData.ObjectRight(MdoReference.create("Catalogs.TestObject"), Collections.emptyList()); | |
| assertThat(objectRight).isNotNull(); | |
| assertThat(objectRight.name().getMdoRef()).isEqualTo("Catalog.TestObject"); | |
| assertThat(objectRight.rights()).isEmpty(); | |
| } | |
| @Test | |
| void testObjectRight() { | |
| var objectRight = new RoleData.ObjectRight(MdoReference.create("Catalog.TestObject"), Collections.emptyList()); | |
| assertThat(objectRight).isNotNull(); | |
| assertThat(objectRight.name().getMdoRef()).isEqualTo("Catalog.TestObject"); | |
| assertThat(objectRight.rights()).isEmpty(); | |
| } |
🤖 Prompt for AI Agents
In src/test/java/com/github/_1c_syntax/bsl/mdo/storage/RoleDataTest.java around
lines 48 to 55, the test constructs an MdoReference with "Catalogs.TestObject"
but asserts the name MdoRef equals "Catalog.TestObject", causing a mismatch; fix
by making the created reference and the assertion use the same value (prefer
changing the constructor call on line 50 to
MdoReference.create("Catalog.TestObject") so both sides use the singular form).
| void testHandlerWithShortPath() { | ||
| var handler = new Handler("Module"); | ||
|
|
||
| assertThat(handler.isEmpty()).isFalse(); | ||
| assertThat(handler.getModuleName()).isEmpty(); | ||
| assertThat(handler.getMethodName()).isEmpty(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "Handler.java" | head -20Repository: 1c-syntax/mdclasses
Length of output: 129
🏁 Script executed:
git ls-files | grep -i handler | grep -E "\.(java|kt)$"Repository: 1c-syntax/mdclasses
Length of output: 541
🏁 Script executed:
rg -l "class Handler" --type javaRepository: 1c-syntax/mdclasses
Length of output: 196
🏁 Script executed:
cat -n ./src/main/java/com/github/_1c_syntax/bsl/mdo/support/Handler.javaRepository: 1c-syntax/mdclasses
Length of output: 2827
🏁 Script executed:
cat -n ./src/test/java/com/github/_1c_syntax/bsl/mdo/support/HandlerTest.javaRepository: 1c-syntax/mdclasses
Length of output: 3295
Add getMethodPath() assertion for test consistency.
The test behavior is correct and intentional: isEmpty() checks if the original path is blank (it's not—"Module" is non-empty), while getModuleName() and getMethodName() extract components by splitting on "." (a path with no "." yields empty components). However, other tests in this class consistently assert getMethodPath() alongside other getters. For consistency, add:
assertThat(handler.getMethodPath()).isEqualTo("Module");🤖 Prompt for AI Agents
In src/test/java/com/github/_1c_syntax/bsl/mdo/support/HandlerTest.java around
lines 77 to 83, the test lacks an assertion for getMethodPath() for consistency
with other tests; add a line asserting that handler.getMethodPath()
isEqualTo("Module") so the test verifies the full original path alongside
isEmpty(), getModuleName(), and getMethodName().
| @Test | ||
| void testSetValueWithValidParameters() { | ||
| var builder = TestClass.builder(); | ||
| TransformationUtils.setValue(builder, "name", "TestName"); | ||
|
|
||
| var result = builder.build(); | ||
| assertThat(result.getName()).isEqualTo("TestName"); | ||
| } | ||
|
|
||
| @Test | ||
| void testSetValueWithListParameter() { | ||
| var builder = TestClassWithList.builder(); | ||
| TransformationUtils.setValue(builder, "items", List.of("item1", "item2")); | ||
|
|
||
| var result = builder.build(); | ||
| assertThat(result.getItems()).containsExactly("item1", "item2"); | ||
| } | ||
|
|
||
| @Test | ||
| void testSetValueWithSingularParameter() { | ||
| var builder = TestClassWithList.builder(); | ||
| TransformationUtils.setValue(builder, "item", "singleItem"); | ||
|
|
||
| var result = builder.build(); | ||
| assertThat(result.getItems()).contains("singleItem"); | ||
| } | ||
|
|
||
| @Test | ||
| void testInvokeWithValidMethod() { | ||
| var mutableObject = new MutableTestClass(); | ||
| TransformationUtils.invoke(mutableObject, "reset"); | ||
|
|
||
| assertThat(mutableObject.isReset()).isTrue(); | ||
| } | ||
|
|
||
| @Test | ||
| void testInvokeWithNonExistentMethod() { | ||
| var mutableObject = new MutableTestClass(); | ||
| // Should not throw, just log error | ||
| TransformationUtils.invoke(mutableObject, "nonExistentMethod"); | ||
|
|
||
| assertThat(mutableObject.isReset()).isFalse(); | ||
| } | ||
|
|
||
| @Test | ||
| void testFieldType() { | ||
| var builder = TestClass.builder(); | ||
| var type = TransformationUtils.fieldType(builder, "name"); | ||
|
|
||
| assertThat(type).isNotNull(); | ||
| assertThat(type.getTypeName()).contains("String"); | ||
| } | ||
|
|
||
| @Test | ||
| void testFieldTypeWithNonExistentField() { | ||
| var builder = TestClass.builder(); | ||
| var type = TransformationUtils.fieldType(builder, "nonExistent"); | ||
|
|
||
| assertThat(type).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void testBuilder() { | ||
| var builder = TransformationUtils.builder(TestClass.class); | ||
|
|
||
| assertThat(builder) | ||
| .isNotNull() | ||
| .isInstanceOf(TestClass.TestClassBuilder.class); | ||
| } | ||
|
|
||
| @Test | ||
| void testBuilderWithClassWithoutBuilder() { | ||
| assertThatThrownBy(() -> TransformationUtils.builder(String.class)) | ||
| .isInstanceOf(IllegalArgumentException.class) | ||
| .hasMessageContaining("Incorrect class"); | ||
| } | ||
|
|
||
| @Test | ||
| void testToBuilderWithClassWithoutToBuilder() { | ||
| var simpleObject = new SimpleTestClass("test"); | ||
| var builder = TransformationUtils.toBuilder(simpleObject); | ||
|
|
||
| assertThat(builder).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void testBuildWithBuilder() { | ||
| var builder = TestClass.builder().name("Test").value(100); | ||
| var result = TransformationUtils.build(builder); | ||
|
|
||
| assertThat(result) | ||
| .isNotNull() | ||
| .isInstanceOf(TestClass.class); | ||
| assertThat(((TestClass) result).getName()).isEqualTo("Test"); | ||
| assertThat(((TestClass) result).getValue()).isEqualTo(100); | ||
| } | ||
|
|
||
| @Test | ||
| void testBuildWithBuilderAndPath() { | ||
| var builder = TestClass.builder().name("Test").value(100); | ||
| var path = Paths.get("test/path"); | ||
| var result = TransformationUtils.build(builder, path); | ||
|
|
||
| assertThat(result) | ||
| .isNotNull() | ||
| .isInstanceOf(TestClass.class); | ||
| } | ||
|
|
||
| @Test | ||
| void testBuildWithInvalidBuilder() { | ||
| var result = TransformationUtils.build("not a builder"); | ||
|
|
||
| assertThat(result).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void testSetValueWithInvalidMethodName() { | ||
| var builder = TestClass.builder(); | ||
| // Should not throw, just log error | ||
| TransformationUtils.setValue(builder, "nonExistentMethod", "value"); | ||
|
|
||
| var result = builder.build(); | ||
| assertThat(result).isNotNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void testFieldTypeWithListParameter() { | ||
| var builder = TestClassWithList.builder(); | ||
| var type = TransformationUtils.fieldType(builder, "items"); | ||
|
|
||
| assertThat(type).isNotNull(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add null parameter tests to verify null-safety behavior.
Given that this PR focuses on null-safety and the AI summary indicates that methods like setValue now accept @Nullable Object value, the test suite should include explicit tests for null parameter handling. This is essential to verify the null-safety contract.
🔎 Suggested null parameter tests to add
+ @Test
+ void testSetValueWithNullValue() {
+ var builder = TestClass.builder();
+ // Should handle null value gracefully
+ TransformationUtils.setValue(builder, "name", null);
+
+ var result = builder.build();
+ assertThat(result.getName()).isNull();
+ }
+
+ @Test
+ void testBuilderWithNullClass() {
+ assertThatThrownBy(() -> TransformationUtils.builder(null))
+ .isInstanceOf(NullPointerException.class);
+ }
+
+ @Test
+ void testToBuilderWithNullObject() {
+ var builder = TransformationUtils.toBuilder(null);
+ assertThat(builder).isNull();
+ }
+
+ @Test
+ void testBuildWithNullBuilder() {
+ var result = TransformationUtils.build(null);
+ assertThat(result).isNull();
+ }📝 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.
| @Test | |
| void testSetValueWithValidParameters() { | |
| var builder = TestClass.builder(); | |
| TransformationUtils.setValue(builder, "name", "TestName"); | |
| var result = builder.build(); | |
| assertThat(result.getName()).isEqualTo("TestName"); | |
| } | |
| @Test | |
| void testSetValueWithListParameter() { | |
| var builder = TestClassWithList.builder(); | |
| TransformationUtils.setValue(builder, "items", List.of("item1", "item2")); | |
| var result = builder.build(); | |
| assertThat(result.getItems()).containsExactly("item1", "item2"); | |
| } | |
| @Test | |
| void testSetValueWithSingularParameter() { | |
| var builder = TestClassWithList.builder(); | |
| TransformationUtils.setValue(builder, "item", "singleItem"); | |
| var result = builder.build(); | |
| assertThat(result.getItems()).contains("singleItem"); | |
| } | |
| @Test | |
| void testInvokeWithValidMethod() { | |
| var mutableObject = new MutableTestClass(); | |
| TransformationUtils.invoke(mutableObject, "reset"); | |
| assertThat(mutableObject.isReset()).isTrue(); | |
| } | |
| @Test | |
| void testInvokeWithNonExistentMethod() { | |
| var mutableObject = new MutableTestClass(); | |
| // Should not throw, just log error | |
| TransformationUtils.invoke(mutableObject, "nonExistentMethod"); | |
| assertThat(mutableObject.isReset()).isFalse(); | |
| } | |
| @Test | |
| void testFieldType() { | |
| var builder = TestClass.builder(); | |
| var type = TransformationUtils.fieldType(builder, "name"); | |
| assertThat(type).isNotNull(); | |
| assertThat(type.getTypeName()).contains("String"); | |
| } | |
| @Test | |
| void testFieldTypeWithNonExistentField() { | |
| var builder = TestClass.builder(); | |
| var type = TransformationUtils.fieldType(builder, "nonExistent"); | |
| assertThat(type).isNull(); | |
| } | |
| @Test | |
| void testBuilder() { | |
| var builder = TransformationUtils.builder(TestClass.class); | |
| assertThat(builder) | |
| .isNotNull() | |
| .isInstanceOf(TestClass.TestClassBuilder.class); | |
| } | |
| @Test | |
| void testBuilderWithClassWithoutBuilder() { | |
| assertThatThrownBy(() -> TransformationUtils.builder(String.class)) | |
| .isInstanceOf(IllegalArgumentException.class) | |
| .hasMessageContaining("Incorrect class"); | |
| } | |
| @Test | |
| void testToBuilderWithClassWithoutToBuilder() { | |
| var simpleObject = new SimpleTestClass("test"); | |
| var builder = TransformationUtils.toBuilder(simpleObject); | |
| assertThat(builder).isNull(); | |
| } | |
| @Test | |
| void testBuildWithBuilder() { | |
| var builder = TestClass.builder().name("Test").value(100); | |
| var result = TransformationUtils.build(builder); | |
| assertThat(result) | |
| .isNotNull() | |
| .isInstanceOf(TestClass.class); | |
| assertThat(((TestClass) result).getName()).isEqualTo("Test"); | |
| assertThat(((TestClass) result).getValue()).isEqualTo(100); | |
| } | |
| @Test | |
| void testBuildWithBuilderAndPath() { | |
| var builder = TestClass.builder().name("Test").value(100); | |
| var path = Paths.get("test/path"); | |
| var result = TransformationUtils.build(builder, path); | |
| assertThat(result) | |
| .isNotNull() | |
| .isInstanceOf(TestClass.class); | |
| } | |
| @Test | |
| void testBuildWithInvalidBuilder() { | |
| var result = TransformationUtils.build("not a builder"); | |
| assertThat(result).isNull(); | |
| } | |
| @Test | |
| void testSetValueWithInvalidMethodName() { | |
| var builder = TestClass.builder(); | |
| // Should not throw, just log error | |
| TransformationUtils.setValue(builder, "nonExistentMethod", "value"); | |
| var result = builder.build(); | |
| assertThat(result).isNotNull(); | |
| } | |
| @Test | |
| void testFieldTypeWithListParameter() { | |
| var builder = TestClassWithList.builder(); | |
| var type = TransformationUtils.fieldType(builder, "items"); | |
| assertThat(type).isNotNull(); | |
| } | |
| @Test | |
| void testSetValueWithValidParameters() { | |
| var builder = TestClass.builder(); | |
| TransformationUtils.setValue(builder, "name", "TestName"); | |
| var result = builder.build(); | |
| assertThat(result.getName()).isEqualTo("TestName"); | |
| } | |
| @Test | |
| void testSetValueWithListParameter() { | |
| var builder = TestClassWithList.builder(); | |
| TransformationUtils.setValue(builder, "items", List.of("item1", "item2")); | |
| var result = builder.build(); | |
| assertThat(result.getItems()).containsExactly("item1", "item2"); | |
| } | |
| @Test | |
| void testSetValueWithSingularParameter() { | |
| var builder = TestClassWithList.builder(); | |
| TransformationUtils.setValue(builder, "item", "singleItem"); | |
| var result = builder.build(); | |
| assertThat(result.getItems()).contains("singleItem"); | |
| } | |
| @Test | |
| void testInvokeWithValidMethod() { | |
| var mutableObject = new MutableTestClass(); | |
| TransformationUtils.invoke(mutableObject, "reset"); | |
| assertThat(mutableObject.isReset()).isTrue(); | |
| } | |
| @Test | |
| void testInvokeWithNonExistentMethod() { | |
| var mutableObject = new MutableTestClass(); | |
| // Should not throw, just log error | |
| TransformationUtils.invoke(mutableObject, "nonExistentMethod"); | |
| assertThat(mutableObject.isReset()).isFalse(); | |
| } | |
| @Test | |
| void testFieldType() { | |
| var builder = TestClass.builder(); | |
| var type = TransformationUtils.fieldType(builder, "name"); | |
| assertThat(type).isNotNull(); | |
| assertThat(type.getTypeName()).contains("String"); | |
| } | |
| @Test | |
| void testFieldTypeWithNonExistentField() { | |
| var builder = TestClass.builder(); | |
| var type = TransformationUtils.fieldType(builder, "nonExistent"); | |
| assertThat(type).isNull(); | |
| } | |
| @Test | |
| void testBuilder() { | |
| var builder = TransformationUtils.builder(TestClass.class); | |
| assertThat(builder) | |
| .isNotNull() | |
| .isInstanceOf(TestClass.TestClassBuilder.class); | |
| } | |
| @Test | |
| void testBuilderWithClassWithoutBuilder() { | |
| assertThatThrownBy(() -> TransformationUtils.builder(String.class)) | |
| .isInstanceOf(IllegalArgumentException.class) | |
| .hasMessageContaining("Incorrect class"); | |
| } | |
| @Test | |
| void testToBuilderWithClassWithoutToBuilder() { | |
| var simpleObject = new SimpleTestClass("test"); | |
| var builder = TransformationUtils.toBuilder(simpleObject); | |
| assertThat(builder).isNull(); | |
| } | |
| @Test | |
| void testBuildWithBuilder() { | |
| var builder = TestClass.builder().name("Test").value(100); | |
| var result = TransformationUtils.build(builder); | |
| assertThat(result) | |
| .isNotNull() | |
| .isInstanceOf(TestClass.class); | |
| assertThat(((TestClass) result).getName()).isEqualTo("Test"); | |
| assertThat(((TestClass) result).getValue()).isEqualTo(100); | |
| } | |
| @Test | |
| void testBuildWithBuilderAndPath() { | |
| var builder = TestClass.builder().name("Test").value(100); | |
| var path = Paths.get("test/path"); | |
| var result = TransformationUtils.build(builder, path); | |
| assertThat(result) | |
| .isNotNull() | |
| .isInstanceOf(TestClass.class); | |
| } | |
| @Test | |
| void testBuildWithInvalidBuilder() { | |
| var result = TransformationUtils.build("not a builder"); | |
| assertThat(result).isNull(); | |
| } | |
| @Test | |
| void testSetValueWithInvalidMethodName() { | |
| var builder = TestClass.builder(); | |
| // Should not throw, just log error | |
| TransformationUtils.setValue(builder, "nonExistentMethod", "value"); | |
| var result = builder.build(); | |
| assertThat(result).isNotNull(); | |
| } | |
| @Test | |
| void testFieldTypeWithListParameter() { | |
| var builder = TestClassWithList.builder(); | |
| var type = TransformationUtils.fieldType(builder, "items"); | |
| assertThat(type).isNotNull(); | |
| } | |
| @Test | |
| void testSetValueWithNullValue() { | |
| var builder = TestClass.builder(); | |
| // Should handle null value gracefully | |
| TransformationUtils.setValue(builder, "name", null); | |
| var result = builder.build(); | |
| assertThat(result.getName()).isNull(); | |
| } | |
| @Test | |
| void testBuilderWithNullClass() { | |
| assertThatThrownBy(() -> TransformationUtils.builder(null)) | |
| .isInstanceOf(NullPointerException.class); | |
| } | |
| @Test | |
| void testToBuilderWithNullObject() { | |
| var builder = TransformationUtils.toBuilder(null); | |
| assertThat(builder).isNull(); | |
| } | |
| @Test | |
| void testBuildWithNullBuilder() { | |
| var result = TransformationUtils.build(null); | |
| assertThat(result).isNull(); | |
| } |
🤖 Prompt for AI Agents
In
src/test/java/com/github/_1c_syntax/bsl/reader/common/TransformationUtilsTest.java
around lines 38 to 169, the test suite lacks coverage for null-parameter
behavior introduced by the null-safety changes; add concise unit tests that call
TransformationUtils.setValue(builder, "name", null), setValue for list field
with null, setValue singular item as null, TransformationUtils.invoke(target,
nullMethodName?) only where applicable, and TransformationUtils.build(null) or
build with a builder that has null fields to assert methods do not throw and
return expected null-safe results (e.g., resulting getters return null or
collections remain empty) and that TransformationUtils.fieldType(builder, null)
returns null; ensure tests assert no exception is thrown and validate the
documented null contract.
| @Test | ||
| void testReadConfigurationWithEmptyPath() { | ||
| var path = Paths.get(""); | ||
| var configuration = MDOReader.readConfiguration(path); | ||
|
|
||
| assertThat(configuration).isNotNull(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -C 5 'readConfiguration.*Path' src/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.javaRepository: 1c-syntax/mdclasses
Length of output: 966
🏁 Script executed:
# Also check the test file to see the full context around line 65 and 138
head -150 src/test/java/com/github/_1c_syntax/bsl/reader/MDOReaderTest.java | tail -100Repository: 1c-syntax/mdclasses
Length of output: 3064
🏁 Script executed:
# Get more details about the MDOReader.readConfiguration method signature and implementation
rg -n -B 2 -A 20 'public static.*readConfiguration' src/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.javaRepository: 1c-syntax/mdclasses
Length of output: 45
🏁 Script executed:
# Search for the implementation that handles empty paths
rg -n -B 5 -A 15 'createReader' src/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.java | head -80Repository: 1c-syntax/mdclasses
Length of output: 3348
🏁 Script executed:
# Look for any special handling or validation of empty paths
rg -n 'Paths\.get\("")|isEmpty|isBlank' src/main/java/com/github/_1c_syntax/bsl/reader/MDOReader.javaRepository: 1c-syntax/mdclasses
Length of output: 159
🏁 Script executed:
# Check Configuration.EMPTY to understand what it is
rg -n -B 2 -A 5 'Configuration.EMPTY|EMPTY.*=' src/main/java --type java | grep -A 5 -B 2 ConfigurationRepository: 1c-syntax/mdclasses
Length of output: 7731
Assert that empty path returns Configuration.EMPTY.
The test only asserts that the result is not null, but should explicitly verify it returns Configuration.EMPTY, consistent with the behavior for non-existent paths (line 130-137). An empty path Paths.get("") triggers FakeReader, which always returns Configuration.EMPTY, so the assertion should be:
assertThat(configuration).isEqualTo(Configuration.EMPTY);🤖 Prompt for AI Agents
In src/test/java/com/github/_1c_syntax/bsl/reader/MDOReaderTest.java around
lines 138 to 144, the test only asserts the configuration is not null for an
empty path, but it should assert the explicit expected value; replace the
non-null assertion with an equality check that configuration equals
Configuration.EMPTY to match behavior of FakeReader and the existing tests for
non-existent paths.
|
1 participant
Описание
Правки для null safe
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Дополнительно
Summary by CodeRabbit
Chores
Refactor
Tests
✏️ Tip: You can customize this high-level summary in your review settings.