Use ArgumentNullException.ThrowIfNull where possible#573
Merged
Daniel-Svensson merged 14 commits intoApr 26, 2026
Conversation
Polyfills.cs is now linked in multiple projects for shared compatibility code. Removed netstandard2.0 target from several projects, focusing on net472, net8.0, and net8.0-windows. Updated Microsoft.Data.SqlClient to 7.0.1 and System.CommandLine to 2.0.7. Minor doc update in EndpointRoutePattern.cs.
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 26 minutes and 2 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR standardizes argument validation across the codebase by replacing manual null/empty checks with .NET guard helpers (ThrowIfNull / ThrowIfNullOrEmpty), links a shared Polyfills.cs into multiple projects, removes netstandard2.0 targets from some projects, and updates tests and changelog to reflect empty-string validation behavior changes. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Daniel-Svensson
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (11)
src/OpenRiaServices.Client/Framework/Authentication/WebAuthenticationService.cs (1)
149-155: 🧹 Nitpick | 🔵 TrivialConsider validating
parametersbeforeInitialize()to fail fast.
Initialize()takes a lock, sets_initialized = true, and creates the domain context via reflection. With the current order, a nullparameterscauses that work (and the side effect of making the service "active") to happen before the argument is rejected. Pre-existing, but trivial to fix while you're here.♻️ Proposed reorder
protected internal override Task<LoginResult> LoginAsync(LoginParameters parameters, CancellationToken cancellationToken) { - this.Initialize(); + ArgumentNullException.ThrowIfNull(parameters); - ArgumentNullException.ThrowIfNull(parameters); + this.Initialize();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/Authentication/WebAuthenticationService.cs` around lines 149 - 155, Move the null-check for parameters to before calling Initialize: in the LoginAsync method call ArgumentNullException.ThrowIfNull(parameters) prior to this.Initialize() so you fail fast and avoid triggering Initialize's side effects (setting _initialized and creating the domain context via reflection) when parameters is null; update the LoginAsync method to perform the argument validation first and then call Initialize.src/OpenRiaServices.Server/Framework/Data/ReflectionDomainServiceDescriptionProvider.cs (1)
400-410:⚠️ Potential issue | 🟡 MinorUnreachable
ThrowIfNull(methodInfo)— base initializer dereferencesmethodInfofirst.The
: base(...)call on Line 401 already evaluatesmethodInfo.Name,methodInfo.ReturnType, and passesmethodInfotoGetMethodParameters/GetAttributeCollection. IfmethodInfoisnull, aNullReferenceExceptionis thrown at the base call (or inside the helpers) before Line 403 ever runs, so callers will never see anArgumentNullExceptionfrom this guard.This is pre-existing, but since the refactor is touching this exact line it's worth either dropping the dead check or actually making it effective (e.g., add
ThrowIfNullguards inside the static helpers, where the parameter is dereferenced for the first reachable time).♻️ Option: enforce the null-check from the helpers so it actually fires
public ReflectionDomainOperationEntry(Type domainServiceType, MethodInfo methodInfo, DomainOperation operation) : base(domainServiceType, methodInfo.Name, operation, methodInfo.ReturnType, GetMethodParameters(methodInfo), GetAttributeCollection(methodInfo)) { - ArgumentNullException.ThrowIfNull(methodInfo); - // Generic methods aren’t supported, and will be caught during DomainServiceDescription validation. if (!methodInfo.IsGenericMethodDefinition) { this._method = DynamicMethodUtility.GetDelegateForMethod(methodInfo); } } @@ private static IEnumerable<DomainOperationParameter> GetMethodParameters(MethodInfo methodInfo) { + ArgumentNullException.ThrowIfNull(methodInfo); ParameterInfo[] actualParameters = methodInfo.GetParameters(); @@ private static AttributeCollection GetAttributeCollection(MethodInfo methodInfo) { + ArgumentNullException.ThrowIfNull(methodInfo); // Get all of the attributes defined on the MethodInfo List<Attribute> attributes = new List<Attribute>(methodInfo.GetCustomAttributes(true).Cast<Attribute>());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Server/Framework/Data/ReflectionDomainServiceDescriptionProvider.cs` around lines 400 - 410, The ArgumentNullException.ThrowIfNull(methodInfo) in ReflectionDomainOperationEntry is unreachable because the base(...) initializer dereferences methodInfo first; remove the dead ThrowIfNull from the constructor and instead add null checks inside the static helpers (GetMethodParameters(MethodInfo) and GetAttributeCollection(MethodInfo)) so they throw ArgumentNullException when methodInfo is null, ensuring callers get the intended exception before any dereference; also ensure any other helper called from the base initializer that reads methodInfo performs the same guard.src/OpenRiaServices.Client/Framework/EntityConflict.cs (1)
33-46:⚠️ Potential issue | 🟡 MinorBehavior change:
currentEntityis now validated unconditionally.Per the XML doc on lines 28-30,
ArgumentNullExceptionwas previously thrown forcurrentEntityonly whenisDeleted == false. MovingArgumentNullException.ThrowIfNull(currentEntity)to line 33 (outside theif (!isDeleted)block) makes it always throw whencurrentEntityis null, even ifisDeleted == true.In practice this is a stricter (and arguably safer) guarantee —
_currentEntityis dereferenced unconditionally inOriginalEntityandResolve()— but it is still a behavior delta from the documented contract. Either intentionally update the<exception>doc to reflect the new contract, or keep the previous conditional check.📝 Suggested doc adjustment if the stricter check is intentional
- /// <exception cref="ArgumentNullException">if <paramref name="isDeleted"/> equals <c>false</c> and - /// <paramref name="storeEntity"/>, <paramref name="currentEntity"/> or <paramref name="propertyNames"/> - /// is null.</exception> + /// <exception cref="ArgumentNullException">if <paramref name="currentEntity"/> is null, or + /// if <paramref name="isDeleted"/> equals <c>false</c> and <paramref name="storeEntity"/> + /// or <paramref name="propertyNames"/> is null.</exception>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/EntityConflict.cs` around lines 33 - 46, The constructor now validates currentEntity unconditionally (ArgumentNullException.ThrowIfNull(currentEntity)) which changes the documented contract; either restore the original conditional behavior by moving the currentEntity null-check inside the if (!isDeleted) block (so currentEntity is only required when isDeleted == false), or update the XML <exception> documentation to state that ArgumentNullException is thrown for currentEntity regardless of isDeleted; locate the constructor in EntityConflict (symbols: currentEntity, _currentEntity, isDeleted, OriginalEntity, Resolve()) and apply one of these two fixes so implementation and docs are consistent.src/OpenRiaServices.Client/Framework/ValidationUtilities.cs (1)
591-606:⚠️ Potential issue | 🟡 Minor
ValidateMethodCall: emptymethodNamenow throwsArgumentException(wasArgumentNullException); update XML doc.
ArgumentException.ThrowIfNullOrEmpty(methodName)throwsArgumentExceptionfor empty strings. The previous code path treated empty asArgumentNullException(consistent with the existing<exception cref="ArgumentNullException">…methodName…null</exception>doc on Lines 591-592). Even though this is aninternalhelper, the same pattern exists in caller-facing helpers (ValidateMethodCall/TryValidateCustomUpdateMethodCall) and the surrounding callers may catchArgumentNullException. Either preserve the prior exception type for empty input or update the XML doc to reflect the new contract.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/ValidationUtilities.cs` around lines 591 - 606, The XML doc for ValidateMethodCall is now inaccurate because the code uses ArgumentException.ThrowIfNullOrEmpty(methodName) which throws ArgumentException for empty strings instead of ArgumentNullException; either revert to checking null only to preserve the original ArgumentNullException behavior or update the XML <exception> tag to document ArgumentException for empty methodName. Locate the internal ValidateMethodCall overloads (methodName, ValidationContext validationContext, object[] parameters, and the ValidateMethodCall wrapper) and change the validation or the XML comment accordingly so the declared exceptions match the actual throw behavior (i.e., replace the ArgumentNullException doc with ArgumentException or replace ThrowIfNullOrEmpty with a null-only check).src/OpenRiaServices.Client/Framework/Authentication/AuthenticationService.cs (1)
455-462:⚠️ Potential issue | 🟠 MajorException-type / XML-doc mismatch in
RaisePropertyChanged.
ArgumentException.ThrowIfNullOrEmpty(propertyName)throwsArgumentExceptionfor empty input, but the XML<exception>still advertisesArgumentNullException. This matches the same issue inWebContextBase.RaisePropertyChanged.All three implementations—
WebContextBase,DomainContext, andAuthenticationService—now useThrowIfNullOrEmpty, so adopting a consistent fix across all three is necessary. Either:
- Update all XML docs to document
ArgumentException(with note on when it's thrown), or- Revert to explicit
ArgumentNullExceptionfor null-only checks if that was the original contract intent.Note:
WebContextBaseTest.cscurrently expectsArgumentNullExceptionfor empty strings (line 242), which conflicts with the actualArgumentExceptionthrown by the code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/Authentication/AuthenticationService.cs` around lines 455 - 462, The XML-doc exception tags for RaisePropertyChanged in AuthenticationService (and the same methods in WebContextBase and DomainContext) are incorrect: the code calls ArgumentException.ThrowIfNullOrEmpty(propertyName) which throws ArgumentException for empty strings, so update the XML <exception> documentation for RaisePropertyChanged (in AuthenticationService.RaisePropertyChanged, WebContextBase.RaisePropertyChanged, and DomainContext.RaisePropertyChanged) to document ArgumentException (not ArgumentNullException) and note it is thrown for null or empty propertyName; also update the unit test expectation in WebContextBaseTest.cs (currently expecting ArgumentNullException for empty strings) to expect ArgumentException to keep behavior and docs consistent.src/OpenRiaServices.Client/Framework/Authentication/WebContextBase.cs (1)
125-132:⚠️ Potential issue | 🔴 CriticalTest will fail due to exception type mismatch; XML documentation is incomplete.
ArgumentException.ThrowIfNullOrEmpty(propertyName)throwsArgumentException(notArgumentNullException) for empty strings. The test atsrc/OpenRiaServices.Client/Test/Client.Test/Authentication/WebContextBaseTest.cs:242calls:ExceptionHelper.ExpectArgumentNullException( () => context.RaisePropertyChangedMock(string.Empty), "propertyName");
ExpectArgumentNullExceptionrequires an exact type match (enforced byExpectExceptionHelper<ArgumentNullException>), so this test will fail whenstring.Emptyis passed. Additionally, the XML documentation (lines 125-127) only mentionsArgumentNullExceptionfor null values, and is now incomplete since the code also rejects empty strings.Either:
- Restore
ArgumentNullExceptionfor the empty case by keeping an explicit check- Or update both the XML
<exception>annotation and the test to expectArgumentExceptionfor empty input🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/Authentication/WebContextBase.cs` around lines 125 - 132, The test expects an ArgumentNullException for empty strings, so change RaisePropertyChanged to explicitly check for null or empty and throw ArgumentNullException when propertyName is null or string.Empty (replace the current ArgumentException.ThrowIfNullOrEmpty call); update the XML <exception> documentation on RaisePropertyChanged to mention it throws ArgumentNullException for null or empty propertyName and keep invoking _propertyChangedEventHandler as before; adjust the mock call RaisePropertyChangedMock reference if needed to locate the method.src/OpenRiaServices.Client.DomainClients.Http/Framework/HttpDomainClientFactory.cs (1)
31-42: 🧹 Nitpick | 🔵 TrivialLGTM — but consider applying the same modernization to the sibling constructor.
The refactor at line 34 looks fine. For consistency with the PR theme, the
Func<Uri, HttpClient>overload at lines 49–53 still uses the older??/throw new ArgumentNullException(...)pattern.♻️ Optional alignment
private protected HttpDomainClientFactory(Uri serverBaseUri, Func<Uri, HttpClient> httpClientFactory) { + ArgumentNullException.ThrowIfNull(httpClientFactory); base.ServerBaseUri = serverBaseUri; - this._httpClientFactory = httpClientFactory ?? throw new ArgumentNullException(nameof(httpClientFactory)); + this._httpClientFactory = httpClientFactory; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client.DomainClients.Http/Framework/HttpDomainClientFactory.cs` around lines 31 - 42, The sibling constructor that accepts Func<Uri, HttpClient> should be modernized to use ArgumentNullException.ThrowIfNull instead of the older "?? throw new ArgumentNullException(...)" pattern; locate the other HttpDomainClientFactory constructor that assigns this._httpClientFactory from the incoming Func<Uri, HttpClient> and replace the null-coalescing check with a call to ArgumentNullException.ThrowIfNull(func) and then assign the func directly to this._httpClientFactory so it matches the null-check style used in the Uri-parameter constructor.src/OpenRiaServices.Client/Framework/ComplexObject.cs (1)
379-384:⚠️ Potential issue | 🟡 MinorXML doc is now stale: empty
propertyNamethrowsArgumentException, notArgumentNullException.
ArgumentException.ThrowIfNullOrEmptythrowsArgumentNullExceptiononly when the value isnull; it throwsArgumentExceptionwhen the string is empty. The<exception>tag onValidatePropertystill claimsArgumentNullExceptionis thrown for bothnulland empty, which is no longer accurate after this refactor. The same applies (implicitly) toRaisePropertyChanged(line 215),RaiseDataMemberChanged(line 237), andRaiseDataMemberChanging(line 303), which previously documented uniform behavior in surrounding APIs.Note that this is a (minor) observable behavior change for any caller doing
catch (ArgumentNullException)on empty input. Worth either fixing the docs or, if strict back‑compat on the exception type is desired, keeping a manualif (string.IsNullOrEmpty(propertyName)) throw new ArgumentNullException(...).📝 Proposed doc fix
- /// <exception cref="ArgumentNullException"> is thrown if <paramref name="propertyName"/> is <c>null</c> or empty.</exception> + /// <exception cref="ArgumentNullException"> is thrown if <paramref name="propertyName"/> is <c>null</c>.</exception> + /// <exception cref="ArgumentException"> is thrown if <paramref name="propertyName"/> is empty.</exception> /// <exception cref="InvalidOperationException"> is thrown if this property is marked with <see cref="EditableAttribute"/> /// configured to prevent editing.</exception>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/ComplexObject.cs` around lines 379 - 384, The XML docs for ValidateProperty (and the similar APIs RaisePropertyChanged, RaiseDataMemberChanged, RaiseDataMemberChanging) are stale: the method now calls ArgumentException.ThrowIfNullOrEmpty which throws ArgumentNullException for null but ArgumentException for empty strings, so update the <exception> tags to document ArgumentException (or explicitly state both ArgumentNullException for null and ArgumentException for empty) to match runtime behavior; alternatively, if you want to preserve the prior behavior, replace the ThrowIfNullOrEmpty call in ValidateProperty (and the other methods) with an explicit check that throws ArgumentNullException for both null and empty — reference the methods ValidateProperty, RaisePropertyChanged, RaiseDataMemberChanged, and RaiseDataMemberChanging when making the change.src/OpenRiaServices.Client/Framework/ComplexObject.INotifyDataErrorInfo.cs (1)
47-64: 🧹 Nitpick | 🔵 TrivialBehavioral change in
GetErrors(null/empty)— document explicitly in PR description and release notes.The predicate has changed from filtering by
MemberNames.Contains(propertyName)(matching only errors that literally contain that entry) to!e.MemberNames.Any() || e.MemberNames.Any(string.IsNullOrEmpty), which additionally includes errors with no member names — matching the type-level shape produced byValidationUtilities.RemoveMemberPrefix(empty list) in addition to the empty-string shape.Since the rest of the PR is documented as "modernisation" (ThrowIfNull replacements), this functional change is easy to miss in review.
INotifyDataErrorInfo.GetErrorsis part of the public binding surface used by UI frameworks, so the behavioral shift warrants explicit mention in the PR description and release notes.Note: Both
EntityandComplexObjectimplementations already use the same predicate logic here, so they remain consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/ComplexObject.INotifyDataErrorInfo.cs` around lines 47 - 64, The GetErrors(null/empty) behavior in ComplexObject.INotifyDataErrorInfo.GetErrors was changed to include ValidationResult entries with either an empty MemberNames collection or a member name that is an empty string (predicate now uses !e.MemberNames.Any() || e.MemberNames.Any(string.IsNullOrEmpty)), which is a functional change vs. the prior Contains-only behavior; update the PR description and release notes to explicitly call this out, reference ComplexObject.INotifyDataErrorInfo.GetErrors and ValidationUtilities.RemoveMemberPrefix (and note that Entity uses the same predicate), and include a short compatibility note describing that callers observing type-level validation errors will now see those results when passing null/empty propertyName.src/OpenRiaServices.Client/Framework/OpenRiaServices.Client.csproj (1)
10-12: 🧹 Nitpick | 🔵 TrivialDead
netstandard2.0ItemGroup after target removal.Now that
netstandard2.0is no longer inTargetFrameworks, the conditionalItemGroupat Lines 10–12 is unreachable and theSystem.ComponentModel.Annotationsreference will never be added. Drop the block to avoid future confusion.♻️ Proposed cleanup
- <ItemGroup Condition=" '$(TargetFramework)' == 'netstandard2.0' "> - <PackageReference Include="System.ComponentModel.Annotations" Version="5.0.0" /> - </ItemGroup> - <ItemGroup Condition=" '$(TargetFramework)' == 'net472' ">🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/OpenRiaServices.Client.csproj` around lines 10 - 12, Remove the now-dead ItemGroup conditional that targets netstandard2.0 by deleting the block with Condition=" '$(TargetFramework)' == 'netstandard2.0' " which contains the PackageReference Include="System.ComponentModel.Annotations" Version="5.0.0"; this cleans up the csproj so the unreachable System.ComponentModel.Annotations reference is removed.src/OpenRiaServices.Client/Framework/DomainContext.cs (1)
859-864: 🧹 Nitpick | 🔵 TrivialInconsistent modernization in
InvokeOperationAsync.The remainder of this PR consistently adopts
ArgumentException.ThrowIfNullOrEmptyfor required strings (e.g.,RaisePropertyChangedat Line 907,ValidateMethodat Line 938), but the manualstring.IsNullOrEmpty(operationName)check at Lines 859–862 was left untouched while the adjacentreturnTypecheck on Line 864 was modernized.If preserving the localized
Resource.Parameter_NullOrEmptymessage is intentional, consider adding a brief comment so future readers don't "fix" the inconsistency. Otherwise, this can be collapsed to:♻️ Proposed unification
- if (string.IsNullOrEmpty(operationName)) - { - throw new ArgumentException(string.Format(CultureInfo.CurrentCulture, Resource.Parameter_NullOrEmpty, "operationName")); - } - - ArgumentNullException.ThrowIfNull(returnType); + ArgumentException.ThrowIfNullOrEmpty(operationName); + ArgumentNullException.ThrowIfNull(returnType);Note:
ArgumentException.ThrowIfNullOrEmptywill use the BCL's default message rather thanResource.Parameter_NullOrEmpty, which is a small loss of localization fidelity for this argument.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/OpenRiaServices.Client/Framework/DomainContext.cs` around lines 859 - 864, The manual null-or-empty check for operationName in InvokeOperationAsync is inconsistent with the rest of the file; replace the string.IsNullOrEmpty(...) + throw new ArgumentException(...) block with a single call to ArgumentException.ThrowIfNullOrEmpty(nameof(operationName)) to match the modernized checks (e.g., RaisePropertyChanged, ValidateMethod), or if you intentionally want to keep the localized Resource.Parameter_NullOrEmpty message, retain the current code but add a brief comment above the check explaining the localization exception so future maintainers don't change it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@src/OpenRiaServices.Client.DomainClients.Http/Framework/OpenRiaServices.Client.DomainClients.Http.csproj`:
- Line 4: The TargetFrameworks element in
OpenRiaServices.Client.DomainClients.Http.csproj was changed to remove
netstandard2.0 which is a breaking change for external consumers; either restore
netstandard2.0 to the TargetFrameworks list (alongside net472 and net8.0) or, if
removal is intentional, update the NuGet release strategy by bumping the major
version and add a migration note in the release notes documenting removal of
netstandard2.0 so downstream consumers (e.g., Xamarin/Mono) are informed.
In `@src/OpenRiaServices.Client/Framework/DomainContext.cs`:
- Around line 905-909: The polyfill ArgumentException.ThrowIfNullOrEmpty is
implemented incorrectly: it currently throws ArgumentNullException for both null
and empty inputs; update its logic in Polyfills.cs so that if value is null it
calls ArgumentNullException.ThrowIfNull (or throws ArgumentNullException), and
if value is empty it calls the helper ThrowArgumentException (so an empty string
produces ArgumentException like the BCL). Keep the existing
ArgumentException.ThrowIfNullOrEmpty method name and ensure it uses
ThrowArgumentException for the empty-case path to match BCL behavior.
In `@src/OpenRiaServices.Client/Framework/EntityConflict.cs`:
- Line 40: Remove the stray empty line between the consecutive ThrowIfNull calls
in the EntityConflict constructor so the two ThrowIfNull invocations are
adjacent (matching the formatting used in other refactored constructors); locate
the constructor in class EntityConflict and delete the extra blank line between
the ThrowIfNull(...) statements.
In `@src/OpenRiaServices.Client/Framework/EntitySet.cs`:
- Line 44: The current polyfill attempted to add ThrowIfNull to
ArgumentNullException which is invalid; instead add a public static method
ThrowIfNull(object value, string? paramName = null) to the Polyfills class and
replace all usages of ArgumentNullException.ThrowIfNull(...) with
Polyfills.ThrowIfNull(...). For example, change the call in EntitySet (the
instance that currently calls ArgumentNullException.ThrowIfNull(entityType)) to
Polyfills.ThrowIfNull(entityType) and update any other files that call the
nonexistent static on ArgumentNullException to call the new
Polyfills.ThrowIfNull method. Ensure the Polyfills.ThrowIfNull signature and
null check logic mirror the expected behavior (throw new
ArgumentNullException(paramName) when value is null).
In `@src/OpenRiaServices.Client/Framework/InvokeArgs.cs`:
- Line 23: The refactor changed exception types for empty operationName; restore
the original behavior in the InvokeArgs constructor by ensuring both null and
empty operationName produce ArgumentNullException (not ArgumentException).
Replace or augment the current call to
ArgumentException.ThrowIfNullOrEmpty(operationName) in the InvokeArgs
constructor with an explicit check that throws new
ArgumentNullException(nameof(operationName)) when
string.IsNullOrEmpty(operationName) is true, so external callers/tests that
expect ArgumentNullException for empty strings continue to work.
In `@src/OpenRiaServices.Client/Framework/InvokeOperation.cs`:
- Line 35: The internal InvokeOperation constructor currently calls
ArgumentException.ThrowIfNullOrEmpty(operationName) which changes the observable
exception from ArgumentNullException to ArgumentException; change the check in
the internal constructor (the ctor used by InvokeOperation<TValue>) to
explicitly throw new ArgumentNullException(nameof(operationName)) when
operationName is null and throw new ArgumentException("Operation name cannot be
empty.", nameof(operationName)) when operationName is empty so callers that
expect ArgumentNullException for null inputs keep the original behavior.
In `@src/OpenRiaServices.Client/Framework/ValidationResultInfo.cs`:
- Line 41: The polyfill uses invalid syntax `extension(ArgumentNullException)`
in Polyfills.cs; replace it with a proper static extension class (e.g.,
ArgumentNullExceptionExtensions) and implement a standard static method
`ThrowIfNull` that takes the argument (object? argument) and optional caller
expression paramName, uses [MethodImpl(MethodImplOptions.AggressiveInlining)]
and [CallerArgumentExpression(nameof(argument))] on the paramName, checks `if
(argument is null)` and calls the existing ThrowArgumentNullException helper;
update references to ArgumentNullException.ThrowIfNull to use the new static
extension class if needed so the net472 conditional still compiles.
In
`@src/OpenRiaServices.Hosting.Wcf.Endpoint/Framework/WCF/Tracing/InMemoryTraceListener.cs`:
- Around line 58-63: The new use of ArgumentNullException.ThrowIfNull(message)
in InMemoryTraceListener.Write causes a compile error on net472 because
Polyfills.cs isn't linked; either add/link Polyfills.cs into this project so
ThrowIfNull is available, or replace the call in the Write method with an
explicit null check (e.g., if (message == null) throw new
ArgumentNullException(nameof(message))). Update the InMemoryTraceListener.Write
implementation accordingly, keeping the currentEntry concatenation logic intact.
In `@src/OpenRiaServices.Hosting.Wcf/Framework/Wcf/DomainServiceHost.cs`:
- Around line 38-39: The project fails to compile on net472 because
DomainServiceHost.cs uses ArgumentNullException.ThrowIfNull (lines shown) but
the shared Polyfills.cs isn't included; open the
OpenRiaServices.Hosting.Wcf.csproj and add a Compile entry in the first
<ItemGroup> of compile items to include the shared Polyfills.cs (and link it as
Polyfills.cs) so the ThrowIfNull polyfill is available at build time.
In `@src/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cs`:
- Around line 41-45: The parameter validation in the DomainOperationEntry
constructor is out of order: it currently checks name first but the signature
declares domainServiceType first; update the
ArgumentNull/ArgumentException.ThrowIfNull* calls in the DomainOperationEntry
constructor so they validate domainServiceType, name, returnType, parameters,
attributes in that exact declaration order (use
ArgumentNullException.ThrowIfNull for domainServiceType and the others as
appropriate) to keep checks consistent with the parameter list.
In `@src/OpenRiaServices.Tools/Framework/ClientCodeGenerationOptions.cs`:
- Line 30: Replace the new ArgumentException.ThrowIfNullOrEmpty(value) call in
the Language setter with an explicit guard that preserves the previous public
contract: check if string.IsNullOrEmpty(value) and if so throw new
ArgumentNullException(nameof(Language), Resource.Null_Language_Property); this
keeps the prior exception type and localized message expected by the test and
callers of the Language setter.
In `@src/OpenRiaServices.Tools/Framework/LinkedServerProjectCache.cs`:
- Around line 36-38: Replace the calls to ArgumentException.ThrowIfNullOrEmpty
in LinkedServerProjectCache (for rootProjectPath and historyFilePath) with
explicit null-or-empty guards that throw ArgumentNullException for both null and
empty inputs to match the existing polyfill behavior; do the same for the
projectPath checks referenced (the other checks at the spots mentioned) and
mirror these explicit-guard changes in ProjectSourceFileCache (the checks at the
listed lines) so the thrown exception type is consistent across target
frameworks and tests expecting ArgumentNullException (reference class/method
names: LinkedServerProjectCache constructor/indexer and ProjectSourceFileCache
methods where ThrowIfNullOrEmpty is used).
In `@src/OpenRiaServices.Tools/Framework/ProjectFileReader.cs`:
- Around line 77-80: The change in ProjectFileReader.LoadProject now rejects
empty projectPath strings via ArgumentException.ThrowIfNullOrEmpty, altering
prior behavior where empty strings fell through to File.Exists("") and returned
the Project_Does_Not_Exist path; revert to only rejecting nulls by using
ArgumentNullException.ThrowIfNull (or equivalent) in LoadProject so empty
strings continue to be handled by the existing File.Exists check, and verify
caller methods GetPropertyValue, LoadProjectReferences, and
LoadSourceFilesFromProject either never pass empty strings or rely on the
soft-fail behavior.
In
`@src/VisualStudio/Tools/Framework/DomainServiceWizard/BusinessLogicContext.cs`:
- Around line 335-337: Restore the previous behavior where null or empty strings
throw ArgumentNullException: keep
ArgumentNullException.ThrowIfNull(codeGenContext) for codeGenContext, and
replace ArgumentException.ThrowIfNullOrEmpty(className) and
ArgumentException.ThrowIfNullOrEmpty(namespaceName) with explicit checks like if
(string.IsNullOrEmpty(className)) throw new
ArgumentNullException(nameof(className)); and similarly for namespaceName so
both null and empty inputs produce ArgumentNullException in
BusinessLogicContext.cs.
In
`@src/VisualStudio/Tools/Framework/DomainServiceWizard/BusinessLogicViewModel.cs`:
- Around line 50-54: Reorder the null/empty checks in the BusinessLogicViewModel
constructor so they follow the constructor parameter order: validate
projectDirectory, className, language, assemblyName, then contextTypes;
specifically move the ArgumentNullException.ThrowIfNull(contextTypes) call to
after ArgumentException.ThrowIfNullOrEmpty(assemblyName) so the validation order
matches the parameter list in the constructor (helping readability and
position-dependent diagnostics).
- Around line 50-54: The production code uses
ArgumentException.ThrowIfNullOrEmpty for projectDirectory, className, language,
and assemblyName (and ArgumentNullException.ThrowIfNull for contextTypes), so
update the unit tests in BusinessLogicClassViewModelTests to expect
ArgumentException for empty-string inputs instead of ArgumentNullException:
replace uses of ExceptionHelper.ExpectArgumentNullExceptionStandard for the
empty-string cases with the appropriate
ExceptionHelper.ExpectArgumentExceptionStandard (or assert ArgumentException)
when invoking the BusinessLogicViewModel constructor or factory that validates
projectDirectory, className, language, and assemblyName; keep null-case
expectations as ArgumentNullException for contextTypes and null parameter tests.
---
Outside diff comments:
In
`@src/OpenRiaServices.Client.DomainClients.Http/Framework/HttpDomainClientFactory.cs`:
- Around line 31-42: The sibling constructor that accepts Func<Uri, HttpClient>
should be modernized to use ArgumentNullException.ThrowIfNull instead of the
older "?? throw new ArgumentNullException(...)" pattern; locate the other
HttpDomainClientFactory constructor that assigns this._httpClientFactory from
the incoming Func<Uri, HttpClient> and replace the null-coalescing check with a
call to ArgumentNullException.ThrowIfNull(func) and then assign the func
directly to this._httpClientFactory so it matches the null-check style used in
the Uri-parameter constructor.
In
`@src/OpenRiaServices.Client/Framework/Authentication/AuthenticationService.cs`:
- Around line 455-462: The XML-doc exception tags for RaisePropertyChanged in
AuthenticationService (and the same methods in WebContextBase and DomainContext)
are incorrect: the code calls ArgumentException.ThrowIfNullOrEmpty(propertyName)
which throws ArgumentException for empty strings, so update the XML <exception>
documentation for RaisePropertyChanged (in
AuthenticationService.RaisePropertyChanged, WebContextBase.RaisePropertyChanged,
and DomainContext.RaisePropertyChanged) to document ArgumentException (not
ArgumentNullException) and note it is thrown for null or empty propertyName;
also update the unit test expectation in WebContextBaseTest.cs (currently
expecting ArgumentNullException for empty strings) to expect ArgumentException
to keep behavior and docs consistent.
In
`@src/OpenRiaServices.Client/Framework/Authentication/WebAuthenticationService.cs`:
- Around line 149-155: Move the null-check for parameters to before calling
Initialize: in the LoginAsync method call
ArgumentNullException.ThrowIfNull(parameters) prior to this.Initialize() so you
fail fast and avoid triggering Initialize's side effects (setting _initialized
and creating the domain context via reflection) when parameters is null; update
the LoginAsync method to perform the argument validation first and then call
Initialize.
In `@src/OpenRiaServices.Client/Framework/Authentication/WebContextBase.cs`:
- Around line 125-132: The test expects an ArgumentNullException for empty
strings, so change RaisePropertyChanged to explicitly check for null or empty
and throw ArgumentNullException when propertyName is null or string.Empty
(replace the current ArgumentException.ThrowIfNullOrEmpty call); update the XML
<exception> documentation on RaisePropertyChanged to mention it throws
ArgumentNullException for null or empty propertyName and keep invoking
_propertyChangedEventHandler as before; adjust the mock call
RaisePropertyChangedMock reference if needed to locate the method.
In `@src/OpenRiaServices.Client/Framework/ComplexObject.cs`:
- Around line 379-384: The XML docs for ValidateProperty (and the similar APIs
RaisePropertyChanged, RaiseDataMemberChanged, RaiseDataMemberChanging) are
stale: the method now calls ArgumentException.ThrowIfNullOrEmpty which throws
ArgumentNullException for null but ArgumentException for empty strings, so
update the <exception> tags to document ArgumentException (or explicitly state
both ArgumentNullException for null and ArgumentException for empty) to match
runtime behavior; alternatively, if you want to preserve the prior behavior,
replace the ThrowIfNullOrEmpty call in ValidateProperty (and the other methods)
with an explicit check that throws ArgumentNullException for both null and empty
— reference the methods ValidateProperty, RaisePropertyChanged,
RaiseDataMemberChanged, and RaiseDataMemberChanging when making the change.
In `@src/OpenRiaServices.Client/Framework/ComplexObject.INotifyDataErrorInfo.cs`:
- Around line 47-64: The GetErrors(null/empty) behavior in
ComplexObject.INotifyDataErrorInfo.GetErrors was changed to include
ValidationResult entries with either an empty MemberNames collection or a member
name that is an empty string (predicate now uses !e.MemberNames.Any() ||
e.MemberNames.Any(string.IsNullOrEmpty)), which is a functional change vs. the
prior Contains-only behavior; update the PR description and release notes to
explicitly call this out, reference ComplexObject.INotifyDataErrorInfo.GetErrors
and ValidationUtilities.RemoveMemberPrefix (and note that Entity uses the same
predicate), and include a short compatibility note describing that callers
observing type-level validation errors will now see those results when passing
null/empty propertyName.
In `@src/OpenRiaServices.Client/Framework/DomainContext.cs`:
- Around line 859-864: The manual null-or-empty check for operationName in
InvokeOperationAsync is inconsistent with the rest of the file; replace the
string.IsNullOrEmpty(...) + throw new ArgumentException(...) block with a single
call to ArgumentException.ThrowIfNullOrEmpty(nameof(operationName)) to match the
modernized checks (e.g., RaisePropertyChanged, ValidateMethod), or if you
intentionally want to keep the localized Resource.Parameter_NullOrEmpty message,
retain the current code but add a brief comment above the check explaining the
localization exception so future maintainers don't change it.
In `@src/OpenRiaServices.Client/Framework/EntityConflict.cs`:
- Around line 33-46: The constructor now validates currentEntity unconditionally
(ArgumentNullException.ThrowIfNull(currentEntity)) which changes the documented
contract; either restore the original conditional behavior by moving the
currentEntity null-check inside the if (!isDeleted) block (so currentEntity is
only required when isDeleted == false), or update the XML <exception>
documentation to state that ArgumentNullException is thrown for currentEntity
regardless of isDeleted; locate the constructor in EntityConflict (symbols:
currentEntity, _currentEntity, isDeleted, OriginalEntity, Resolve()) and apply
one of these two fixes so implementation and docs are consistent.
In `@src/OpenRiaServices.Client/Framework/OpenRiaServices.Client.csproj`:
- Around line 10-12: Remove the now-dead ItemGroup conditional that targets
netstandard2.0 by deleting the block with Condition=" '$(TargetFramework)' ==
'netstandard2.0' " which contains the PackageReference
Include="System.ComponentModel.Annotations" Version="5.0.0"; this cleans up the
csproj so the unreachable System.ComponentModel.Annotations reference is
removed.
In `@src/OpenRiaServices.Client/Framework/ValidationUtilities.cs`:
- Around line 591-606: The XML doc for ValidateMethodCall is now inaccurate
because the code uses ArgumentException.ThrowIfNullOrEmpty(methodName) which
throws ArgumentException for empty strings instead of ArgumentNullException;
either revert to checking null only to preserve the original
ArgumentNullException behavior or update the XML <exception> tag to document
ArgumentException for empty methodName. Locate the internal ValidateMethodCall
overloads (methodName, ValidationContext validationContext, object[] parameters,
and the ValidateMethodCall wrapper) and change the validation or the XML comment
accordingly so the declared exceptions match the actual throw behavior (i.e.,
replace the ArgumentNullException doc with ArgumentException or replace
ThrowIfNullOrEmpty with a null-only check).
In
`@src/OpenRiaServices.Server/Framework/Data/ReflectionDomainServiceDescriptionProvider.cs`:
- Around line 400-410: The ArgumentNullException.ThrowIfNull(methodInfo) in
ReflectionDomainOperationEntry is unreachable because the base(...) initializer
dereferences methodInfo first; remove the dead ThrowIfNull from the constructor
and instead add null checks inside the static helpers
(GetMethodParameters(MethodInfo) and GetAttributeCollection(MethodInfo)) so they
throw ArgumentNullException when methodInfo is null, ensuring callers get the
intended exception before any dereference; also ensure any other helper called
from the base initializer that reads methodInfo performs the same guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c0396635-ca03-4efe-940b-51081091dc6a
📒 Files selected for processing (121)
src/.editorconfigsrc/OpenRiaServices.Client.DomainClients.Http/Framework/Http/DataContractHttpDomainClient.cssrc/OpenRiaServices.Client.DomainClients.Http/Framework/HttpDomainClientFactory.cssrc/OpenRiaServices.Client.DomainClients.Http/Framework/OpenRiaServices.Client.DomainClients.Http.csprojsrc/OpenRiaServices.Client.DomainClients.Http/Framework/SystemServiceModel.Dispatcher/QueryStringConverter.cssrc/OpenRiaServices.Client.Web/Framework/Data/ServiceQueryPart.cssrc/OpenRiaServices.Client.Web/Framework/Data/WebDomainClient.cssrc/OpenRiaServices.Client.Web/Framework/OpenRiaServices.Client.Web.csprojsrc/OpenRiaServices.Client.Web/Framework/Web/WcfDomainClientFactory.cssrc/OpenRiaServices.Client/Framework/Authentication/AuthenticationEventArgs.cssrc/OpenRiaServices.Client/Framework/Authentication/AuthenticationService.cssrc/OpenRiaServices.Client/Framework/Authentication/WebAuthenticationService.cssrc/OpenRiaServices.Client/Framework/Authentication/WebContextBase.cssrc/OpenRiaServices.Client/Framework/ComplexObject.INotifyDataErrorInfo.cssrc/OpenRiaServices.Client/Framework/ComplexObject.cssrc/OpenRiaServices.Client/Framework/DomainClient.cssrc/OpenRiaServices.Client/Framework/DomainClientFactory.cssrc/OpenRiaServices.Client/Framework/DomainContext.cssrc/OpenRiaServices.Client/Framework/Entity.cssrc/OpenRiaServices.Client/Framework/EntityChangeSet.cssrc/OpenRiaServices.Client/Framework/EntityCollection.cssrc/OpenRiaServices.Client/Framework/EntityCollectionChangedEventArgs.cssrc/OpenRiaServices.Client/Framework/EntityConflict.cssrc/OpenRiaServices.Client/Framework/EntityContainer.cssrc/OpenRiaServices.Client/Framework/EntityKey.cssrc/OpenRiaServices.Client/Framework/EntityList.cssrc/OpenRiaServices.Client/Framework/EntityQuery.cssrc/OpenRiaServices.Client/Framework/EntityQueryable.cssrc/OpenRiaServices.Client/Framework/EntityRef.cssrc/OpenRiaServices.Client/Framework/EntitySet.cssrc/OpenRiaServices.Client/Framework/EntityVisitor.cssrc/OpenRiaServices.Client/Framework/InvokeArgs.cssrc/OpenRiaServices.Client/Framework/InvokeCompletedResult.cssrc/OpenRiaServices.Client/Framework/InvokeOperation.cssrc/OpenRiaServices.Client/Framework/LoadOperation.cssrc/OpenRiaServices.Client/Framework/OpenRiaServices.Client.csprojsrc/OpenRiaServices.Client/Framework/OperationBase.cssrc/OpenRiaServices.Client/Framework/QueryBuilder.cssrc/OpenRiaServices.Client/Framework/QueryCompletedResult.cssrc/OpenRiaServices.Client/Framework/SubmitCompletedResult.cssrc/OpenRiaServices.Client/Framework/SubmitOperation.cssrc/OpenRiaServices.Client/Framework/ValidationResultCollection.cssrc/OpenRiaServices.Client/Framework/ValidationResultInfo.cssrc/OpenRiaServices.Client/Framework/ValidationUtilities.cssrc/OpenRiaServices.EntityFramework/Framework/DbContextExtensions.cssrc/OpenRiaServices.EntityFramework/Framework/DbDomainServiceDescriptionProviderAttribute.cssrc/OpenRiaServices.EntityFramework/Framework/LinqToEntitiesDomainServiceDescriptionProviderAttribute.cssrc/OpenRiaServices.EntityFramework/Framework/LinqToEntitiesTypeDescriptionContext.cssrc/OpenRiaServices.EntityFramework/Framework/MetadataWorkspaceUtilities.cssrc/OpenRiaServices.EntityFramework/Framework/ObjectContextExtensions.cssrc/OpenRiaServices.EntityFramework/Framework/ObjectContextUtilities.cssrc/OpenRiaServices.EntityFramework/Framework/OpenRiaServices.EntityFramework.csprojsrc/OpenRiaServices.Hosting.Local/Framework/DomainServiceProxy.cssrc/OpenRiaServices.Hosting.Local/Framework/Internal/DomainServiceProxyGenerator.cssrc/OpenRiaServices.Hosting.Local/Framework/OpenRiaServices.Hosting.Local.csprojsrc/OpenRiaServices.Hosting.Wcf.Endpoint/Framework/WCF/Tracing/InMemoryTraceListener.cssrc/OpenRiaServices.Hosting.Wcf.Endpoint/Framework/WCF/Tracing/TracingDomainServiceEndpointFactory.cssrc/OpenRiaServices.Hosting.Wcf/Framework/Linq/DynamicQueryable.cssrc/OpenRiaServices.Hosting.Wcf/Framework/Linq/QueryDeserializer.cssrc/OpenRiaServices.Hosting.Wcf/Framework/Wcf/DomainServiceEndpointFactory.cssrc/OpenRiaServices.Hosting.Wcf/Framework/Wcf/DomainServiceHost.cssrc/OpenRiaServices.Hosting.Wcf/Framework/Wcf/MessageEncoders/PoxBinaryMessageEncodingBindingElement.cssrc/OpenRiaServices.LinqToSql/Framework/DataContextExtensions.cssrc/OpenRiaServices.LinqToSql/Framework/LinqToSqlDomainServiceDescriptionProviderAttribute.cssrc/OpenRiaServices.LinqToSql/Framework/LinqToSqlTypeDescriptionContext.cssrc/OpenRiaServices.LinqToSql/Framework/OpenRiaServices.LinqToSql.csprojsrc/OpenRiaServices.Server.EntityFrameworkCore/Framework/DbContextEFCoreExtensions.cssrc/OpenRiaServices.Server.EntityFrameworkCore/Framework/EFCoreTypeDescriptionContext.cssrc/OpenRiaServices.Server.EntityFrameworkCore/Framework/OpenRiaServices.Server.EntityFrameworkCore.csprojsrc/OpenRiaServices.Server.EntityFrameworkCore/Test/DbContextModel/EFCoreModels.csprojsrc/OpenRiaServices.Server.UnitTesting/Framework/DomainServiceTestHost.cssrc/OpenRiaServices.Server.UnitTesting/Framework/OpenRiaServices.Server.UnitTesting.csprojsrc/OpenRiaServices.Server.UnitTesting/Framework/OperationContext.cssrc/OpenRiaServices.Server/Framework/Authentication/AuthenticationCodeProcessor.cssrc/OpenRiaServices.Server/Framework/Data/AuthorizationAttribute.cssrc/OpenRiaServices.Server/Framework/Data/AuthorizationContext.cssrc/OpenRiaServices.Server/Framework/Data/ChangeSet.cssrc/OpenRiaServices.Server/Framework/Data/ChangeSetEntry.cssrc/OpenRiaServices.Server/Framework/Data/CodeProcessor.cssrc/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cssrc/OpenRiaServices.Server/Framework/Data/DomainOperationParameter.cssrc/OpenRiaServices.Server/Framework/Data/DomainService.cssrc/OpenRiaServices.Server/Framework/Data/DomainServiceDescription.cssrc/OpenRiaServices.Server/Framework/Data/DomainServiceDescriptionProvider.cssrc/OpenRiaServices.Server/Framework/Data/DomainServiceDescriptionProviderAttribute.cssrc/OpenRiaServices.Server/Framework/Data/DomainTypeDescriptionProvider.cssrc/OpenRiaServices.Server/Framework/Data/InvokeDescription.cssrc/OpenRiaServices.Server/Framework/Data/QueryDescription.cssrc/OpenRiaServices.Server/Framework/Data/ReflectionDomainServiceDescriptionProvider.cssrc/OpenRiaServices.Server/Framework/Data/ServiceInvokeResult.cssrc/OpenRiaServices.Server/Framework/Data/ServiceQueryResult.cssrc/OpenRiaServices.Tools.CodeGenTask/OpenRiaServices.Tools.CodeGenTask.csprojsrc/OpenRiaServices.Tools.TextTemplate/Framework/CSharpGenerators/AttributeGenerationHelpers/AttributeGeneratorHelper.cssrc/OpenRiaServices.Tools.TextTemplate/Framework/OpenRiaServices.Tools.TextTemplate.csprojsrc/OpenRiaServices.Tools/Framework/ClientCodeGenerationOptions.cssrc/OpenRiaServices.Tools/Framework/ClientProxyFixupCodeDomVisitor.cssrc/OpenRiaServices.Tools/Framework/CodeDomClientCodeGenerator.cssrc/OpenRiaServices.Tools/Framework/CodeDomVisitor.cssrc/OpenRiaServices.Tools/Framework/CodeGenUtilities.cssrc/OpenRiaServices.Tools/Framework/DomainServiceCatalog.cssrc/OpenRiaServices.Tools/Framework/EndpointRoutePattern.cssrc/OpenRiaServices.Tools/Framework/LinkedServerProjectCache.cssrc/OpenRiaServices.Tools/Framework/MetadataPipeline/AttributeDeclaration.cssrc/OpenRiaServices.Tools/Framework/MetadataPipeline/CustomAttributeGenerator.cssrc/OpenRiaServices.Tools/Framework/MetadataPipeline/StandardCustomAttributeBuilder.cssrc/OpenRiaServices.Tools/Framework/OpenRiaServices.Tools.csprojsrc/OpenRiaServices.Tools/Framework/ProjectFileReader.cssrc/OpenRiaServices.Tools/Framework/ProjectSourceFileCache.cssrc/OpenRiaServices.Tools/Framework/RiaClientFilesTask.cssrc/OpenRiaServices.Tools/Framework/SharedTypes/SharedAssemblies.cssrc/OpenRiaServices.Tools/Framework/SharedTypes/SourceFileLocationService.cssrc/OpenRiaServices.Tools/Framework/ValidateDomainServicesTask.cssrc/OpenRiaServices.Tools/Framework/Validation/DomainServiceValidator.cssrc/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/CrossDomainServiceQueryTests.cssrc/VisualStudio/Tools/Framework/DomainServiceWizard/BusinessLogicContext.cssrc/VisualStudio/Tools/Framework/DomainServiceWizard/BusinessLogicModel.cssrc/VisualStudio/Tools/Framework/DomainServiceWizard/BusinessLogicViewModel.cssrc/VisualStudio/Tools/Framework/DomainServiceWizard/DomainServiceFixupCodeDomVisitor.cssrc/VisualStudio/Tools/Framework/DomainServiceWizard/GeneratedCode.cssrc/VisualStudio/Tools/Framework/OpenRiaServices.VisualStudio.DomainServices.Tools.csprojsrc/VisualStudio/Tools/Framework/WebConfigUtil.cs
💤 Files with no reviewable changes (1)
- src/.editorconfig
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Test/Desktop/OpenRiaServices.Common.Test/ExceptionHelper.cs`:
- Around line 271-275: ExpectEmptyStringArgumentException currently asserts
exception type and ParamName only; update it to also assert the exception
message for parity with ExpectArgumentNullExceptionStandard by either comparing
ex.Message to new ArgumentException("The value cannot be an empty string.",
paramName).Message or asserting the message contains "The value cannot be an
empty string."; modify the ExpectEmptyStringArgumentException method to include
this additional assertion while keeping the existing Assert.ThrowsExactly and
ParamName check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9d1e9609-6690-4bbc-9752-ad08531ebccc
📒 Files selected for processing (8)
Changelog.mdsrc/OpenRiaServices.Client/Framework/EntityConflict.cssrc/OpenRiaServices.Client/Framework/Polyfills.cssrc/OpenRiaServices.Client/Test/Client.Test/Authentication/WebContextBaseTest.cssrc/OpenRiaServices.Server/Framework/Data/DomainOperationEntry.cssrc/Test/Desktop/OpenRiaServices.Common.Test/ExceptionHelper.cssrc/Test/OpenRiaservices.EndToEnd.Wcf.Test/Data/EntityQueryTests.cssrc/VisualStudio/Tools/Framework/DomainServiceWizard/BusinessLogicViewModel.cs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Test/Desktop/OpenRiaServices.Common.Test/ExceptionHelper.cs`:
- Around line 268-273: The test ExpectEmptyStringArgumentException currently
asserts the localized message text "The value cannot be an empty string." which
will fail on non-English builds; update this helper to gate the message-text
assertion behind UnitTestHelper.EnglishBuildAndOS (matching other helpers like
ExpectArgumentException and ExpectArgumentOutOfRangeException) or remove the
message check entirely so it only verifies the exception type and ex.ParamName
in ExpectEmptyStringArgumentException.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e92f1dcb-e076-4c67-8b37-8f34fb320577
📒 Files selected for processing (4)
src/OpenRiaServices.Tools/Test/ClientCodeGenerationOptionsTests.cssrc/OpenRiaServices.Tools/Test/LinkedServerProjectCacheTests.cssrc/OpenRiaServices.Tools/Test/ProjectSourceFileCacheTests.cssrc/Test/Desktop/OpenRiaServices.Common.Test/ExceptionHelper.cs
|
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.
Summary by CodeRabbit
Refactor
Breaking Changes
Chores