+semver:minor Migrate core library to use nullable reference types - #169
Open
imnasnainaec wants to merge 7 commits into
Open
+semver:minor Migrate core library to use nullable reference types #169imnasnainaec wants to merge 7 commits into
imnasnainaec wants to merge 7 commits into
Conversation
…o 9, and add PolySharp to be able to use nullible attributes.
* Address NRT review feedback and clear remaining warnings - MapToExistingLanguageIfPossible now returns the empty string (not null) for empty input, honoring its [return: NotNullIfNotNull] contract. - Removed the incorrect [MemberNotNullWhen(false, Id, Source, Target)] from XLiffTransUnit.IsEmpty: IsEmpty is a conjunction, so !IsEmpty does not imply all three members are non-null. Adjusted ToString and XLiffBody.AddTransUnitRaw, which relied on that bogus guarantee. - DefaultInstalledStringFilePath now throws a clear InvalidOperationException instead of passing a possibly-null folder to Path.Combine. - StringCache is backed by a nullable field and throws a clear InvalidOperationException instead of being `null!` and NREing when the minimal constructor was used. - MergeXliffDocuments hoists the trans-unit id once per iteration and skips units without one, replacing ten null-forgiving `tu.Id!` uses. - Fixed the four remaining warnings: CS8604 in XLiffBody (hoisted documented locals) and CS8618 for DefaultStringFilePath (defaults to string.Empty). Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bring in the pseudolocalization (qps-ploc) support and the BingTranslator replacement, and give the code they added the same nullable-reference-type treatment as the rest of the core project: - PseudoLocalization.Transform / LocalizationManager.PseudoLocalize are annotated string? -> string? with NotNullIfNotNull, matching what they already do (and what PseudoLocalize_NullOrEmpty_ReturnedAsIs asserts). - The pseudo-locale paths that must return non-null now say what they fall back to when neither the caller nor the English cache has any text, using the same fallback the surrounding non-pseudo path uses: string.Empty in GetDynamicStringOrEnglish and XliffLocalizationManager.GetLocalizedString. Previously these returned null from methods declared to return a string. GetStringFromStringCache / GetTooltipFromStringCache keep returning null, which is how the WinForms localizers know to leave the designer text alone. - The translators moved into the core project are annotated for the nulls they already produce and consume: TranslateText / InternalTranslate / PostTranslateProcess return string?, RemoveAcceleratorAmpersands is NotNullIfNotNull, MicrosoftTranslator's key/region config is string? (Environment.GetEnvironmentVariable can return null), and the members deserialized from the Azure and MyMemory JSON payloads are all nullable. Conflict resolutions of note: - MapToExistingLanguageIfPossible keeps the branch's null/empty handling and adds master's pseudo-locale bail-out. - GetDynamicStringOrEnglish takes master's EnglishTextOrFallback helper, which subsumes the branch's inline englishText handling; the helper's englishText parameter is nullable, as its callers' is. - L10NSharp.csproj keeps PolySharp alongside master's dependency upgrades. - The CHANGELOG entry for NRT moves to [Unreleased], since master has since cut 11.0.0, and now names the public annotation change (GetLocalizedString) rather than implying the internal interfaces affect consumers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
imnasnainaec
marked this pull request as ready for review
September 9, 2026 13:20
imnasnainaec
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #157, merging in
master(conceptually applying NRT to all new code, not just mechanically resolving the 3 files with merge conflicts).Devin review: https://app.devin.ai/review/sillsdev/l10nsharp/pull/169
This change is