Fix/assessment findings#15
Merged
Merged
Conversation
…ys; neutral base file is loaded at runtime
C1: GetAllStrings(includeParentCultures: false) was calling LoadResources (merged hierarchy), so
it returned parent/en fallback strings identically to true. Now calls new
LoadResourcesForCultureOnly which loads only {baseName}.{culture}.json with no hierarchy merge and
its own cache prefix ("single:").
C2: LoadResourcesInternal now loads the neutral {baseName}.json file first as the lowest-priority
base layer before applying the culture hierarchy merge, so keys in neutral files are no longer
silently missing at runtime. The C1 single-culture path intentionally does NOT include the neutral
file, matching the contract.
Adds 7 new tests (3 C1, 2 C2, 2 C1+C2 interaction) and updates the GetAllStrings XML doc comment.
Full suite: 127 tests passing (120 existing + 7 new).
…n fixture The brief asked to update GetAllStrings_WithIncludeParentCulturesFalse_ReturnsAllStrings to the corrected contract. Its premise (asserts parent strings when false) does not hold for the en fixture (en is its own fallback, so single-culture == merged). Renamed to GetAllStrings_WithIncludeParentCulturesFalse_ReturnsOnlySingleCultureFile and added ResourceNotFound assertion. Discriminating fallback-exclusion proof lives in the es and fr tests.
…analyzer X1: ExtractKeyFromArgument now uses argumentOperation.ConstantValue to extract keys from any compile-time constant (const field refs, folded expressions) instead of only ILiteralOperation, so generated const keys are no longer reported as unused (LOC002) or silently skipped (LOC001). The now-unused IsStringLiteralConstant helper is removed. A3: IsTypeOrImplementsInterface replaces every .Contains(typeName) with .Equals(typeName, StringComparison.Ordinal), preventing superstring type names like IStringLocalizerExtensions from falsely matching the IStringLocalizer accessor config.
…talog X2: ExtractKeysFromJsonElement now recurses into JSON arrays, producing indexed keys (items[0], list[0].name) matching FlattenJsonElement exactly. Array containers (items) are no longer emitted as leaf keys. A2: InferCultureFromPath uses exact token equality instead of IndexOf substring matching. Filename suffix checked first, then directory segments — never substring-within-a-segment. Prevents Resources/ → es and Presentation.en.json → es misclassifications. Adds JsonKeyCatalogFixTests.cs with 10 tests covering both fixes (TDD). Full suite: 105/105 passing.
…d-key tracking The analyzer enables EnableConcurrentExecution() but was mutating a plain HashSet<string> usedKeys from concurrently-invoked operation callbacks, causing potential data races, dropped entries, and nondeterministic LOC002. - Replace HashSet<string> with ConcurrentDictionary<string, byte> using the same case-sensitivity comparer (Ordinal or OrdinalIgnoreCase). - Record usage via TryAdd(key, 0) instead of Add(key). - Simplify ReportUnusedKeys to usedKeys.ContainsKey(key) — the dictionary's comparer handles both case-sensitive and case-insensitive matching, removing the Any(uk => string.Equals(...)) branch and the config parameter. - Add TestAnalyzerConfigOptions/Provider helpers and a CodeVerifier overload to support custom editorconfig options in tests. - Add CaseInsensitive_UsedKeyWithDifferentCasing_DoesNotProduce_LOC002 test to guard case-insensitive comparer behavior through the new collection. All 106 tests pass.
…erator The runtime (JsonResourceLoader.FlattenJsonElement) never produces bare array-container keys like "items" — only indexed entries "items[0]", "items[1]", etc. After X2 aligned the analyzer catalog with the runtime, any generated Res.Items.Key reference produced LOC001 (Error, build break). Remove the Key = "..." constant from both the top-level array case and the nested-array case in SourceEmitter.cs. All indexed _ItemN constants and nested ItemN classes remain. Update 3 existing tests that asserted Key was emitted; add 1 new focused regression test.
…ssingCultures
After C2, the runtime merges {baseName}.json (the neutral file, cataloged
under "default") into every culture's resolution as a shared base layer.
A key present only in the neutral file therefore resolves in ALL cultures
at runtime — but the analyzer was still flagging it as partially missing
(LOC003) because GetMissingCultures only checked per-culture buckets.
Add an early-return guard at the top of GetMissingCultures: if the key
exists in _keysByCulture["default"], return an empty missing list. The
en-fallback path is unaffected — a key only in "en" still produces LOC003
for "fr". Update DefaultCulture_VsRealCultures_HandlingInLOC003 (was
encoding the pre-C2 false positive); add 2 new regression tests.
Replace hardcoded `new CultureInfo("en")` in `GetCultureHierarchy` with a
configurable `JsonLocalizationOptions.FallbackCulture` property (default "en",
null/empty disables). Adds optional `fallbackCulture` parameter to the
`JsonResourceLoader` constructor and the file-provider DI overload. Threads the
option through the options-based DI factory. Adds 17 new tests (TDD) covering
custom fallback, disabled fallback, precedence, fast-fail on invalid culture,
single-culture path isolation, and DI wiring. All 144 tests pass.
…s, and consolidate frameworks into Directory.Build.props
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
1 participant
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.
No description provided.