allow options to be overriden

[david-driscoll]

No description provided.

[codacy-production]

Not up to standards ⛔

🔴 Issues 1 critical

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category	Results
ErrorProne	1 critical

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric	Results
Complexity	0
Duplication	0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

[github-actions]

Test Results

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
0 files   ±0   0 ❌ ±0 

Results for commit 5012eea. ± Comparison against base commit 373a597.

[codacy-production]

Pull Request Overview

The PR aims to allow overriding options and marked locations, but the implementation is currently inconsistent. While metadata is now overwritten using SetItem, the actual source content in AddMarkup is still being appended, which will lead to duplicate file definitions in the resulting compilation (e.g., CS0101).

Additionally, the code is currently not up to standards due to a high-severity risk in OptionsProvider.cs where a potential null reference could occur during property initialization. There is also no test coverage provided for the overriding logic or the refactored provider, which is critical for verifying that the new behavior works as intended without regressing existing functionality.

About this PR

• The PR description is missing, providing no context on the motivation for these changes.

Test suggestions

• Invoke AddOption twice for the same path and key; verify the resulting context contains the second value.
• Invoke AddGlobalOption twice for the same key; verify the second value overwrites the first.
• Invoke AddMarkup twice with the same name; verify the _markedLocations dictionary contains the location/trigger from the second call.
• Verify that OptionsProvider correctly merges global options into file options without overwriting existing file-specific keys.

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Invoke `AddOption` twice for the same path and key; verify the resulting context contains the second value.
2. Invoke `AddGlobalOption` twice for the same key; verify the second value overwrites the first.
3. Invoke `AddMarkup` twice with the same name; verify the `_markedLocations` dictionary contains the location/trigger from the second call.
4. Verify that `OptionsProvider` correctly merges global options into file options without overwriting existing file-specific keys.

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

_{🔴 HIGH RISK}

The _properties field is initialized from the primary constructor parameter without a null check. If properties is null, any call to TryGetValue will result in a NullReferenceException. Consider initializing with a null check: private readonly ImmutableDictionary<string, string> _properties = properties ?? ImmutableDictionary<string, string>.Empty;.

See Issue in Codacy

[codacy-production]

_{🟡 MEDIUM RISK}

While _markedLocations was updated to SetItem to allow overriding, _sources remains additive. To fully support overriding markup, the existing source with the same name must be replaced in the _sources collection; otherwise, the compilation will contain multiple files with the same name, leading to 'Duplicate type defined' compiler errors (e.g., CS0101).

[codacy-production]

_{⚪ LOW RISK}

Suggestion: This merge logic can be simplified and made more efficient by using SetItems to apply local overrides to the global options.

Suggested change

	if (!options.TryGetValue(tree.FilePath, out var value)) return GlobalOptions;
	foreach (var v in globalOptions.Where(v => !value.ContainsKey(v.Key)))
	{
	value = value.Add(v.Key, v.Value);
	value = value.SetItem(v.Key, v.Value);
	}
	return new OptionsObject(value);
	if (!options.TryGetValue(tree.FilePath, out var value)) return GlobalOptions;
	return new OptionsObject(globalOptions.SetItems(value));

[codacy-production]

_{⚪ LOW RISK}

Suggestion: This logic can be simplified to a single expression using GetValueOrDefault.

Suggested change

	var rootOptions = _options;
	if (rootOptions.TryGetValue(path, out var fileOptions))
	if (_options.TryGetValue(path, out var fileOptions))
	{
	fileOptions = fileOptions.Add(key, value);
	fileOptions = fileOptions.SetItem(key, value);
	rootOptions = rootOptions.SetItem(path, fileOptions);
	}
	else
	{
	rootOptions = rootOptions.SetItem(path, ImmutableDictionary<string, string>.Empty.Add(key, value));
	rootOptions = rootOptions.SetItem(path, ImmutableDictionary<string, string>.Empty.SetItem(key, value));
	}
	return this with { _options = rootOptions, };
	var fileOptions = _options.GetValueOrDefault(path, ImmutableDictionary<string, string>.Empty).SetItem(key, value);
	return this with { _options = _options.SetItem(path, fileOptions) };