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.
Refactor GenerateCommand into focused modules and stabilize package-reference builds #1095
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Refactor GenerateCommand into focused modules and stabilize package-reference builds #1095
Changes from all commits
bb22f8861ccec66f4c2221f2bd7d06a41949ddff1000b8d39File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
In System.Text.Json for modern .NET, what doesJsonSerializer.Deserialize("null")return for a reference typeT? Does it throw, or returnnull?💡 Result:
In System.Text.Json, calling JsonSerializer.Deserialize("null") for a reference type T will return null [1][2]. It does not throw an exception because "null" is considered valid JSON [1][2]. This behavior occurs because the JSON string "null" is a valid JSON value that represents a null reference in.NET [1][2]. Consequently, the deserializer returns a null reference of type T [2]. This is why the return type of the Deserialize method is marked as nullable (T?) in the.NET API documentation; the method must account for the possibility of returning null when the input JSON is explicitly "null" [1][2].
Citations:
🏁 Script executed:
Repository: christianhelle/refitter
Length of output: 188
🏁 Script executed:
Repository: christianhelle/refitter
Length of output: 6611
🏁 Script executed:
Repository: christianhelle/refitter
Length of output: 372
🏁 Script executed:
Repository: christianhelle/refitter
Length of output: 3606
🏁 Script executed:
Repository: christianhelle/refitter
Length of output: 9193
🏁 Script executed:
Repository: christianhelle/refitter
Length of output: 360
🌐 Web query:
In .NET, what happens when calling System.IO.Path.IsPathRooted with a null string argument? Does it throw or return false?💡 Result:
Calling System.IO.Path.IsPathRooted with a null string argument throws an ArgumentNullException; it does not return false. [1][2] In the current.NET implementation, the string overload uses ArgumentNullException.ThrowIfNull(path) before checking whether the path contains a root. [1] Microsoft Learn’s API docs also state that the method’s string overload throws ArgumentNullException when path is null. [2]
Citations:
🌐 Web query:
In .NET, what happens when calling System.IO.Path.Combine with a null segment (e.g., Path.Combine("base", (string)null))? Does it throw ArgumentNullException?💡 Result:
Yes—System.IO.Path.Combine throws ArgumentNullException when any path segment argument it receives is null (including calls like Path.Combine("base", (string)null)). Microsoft’s API docs state that path1 or path2 is null as a parameter error for Path.Combine, which corresponds to ArgumentNullException.[1][2] In other words: - Path.Combine("base", (string)null) => ArgumentNullException (for the null segment argument, e.g., path2). [1][2] Note: This is for the string-based overloads described in the docs (Combine(String, String), and the multi-parameter overloads similarly validate their inputs).
Citations:
🌐 Web query:
In .NET, what does Uri.TryCreate return when the input string is null? Does it throw?💡 Result:
Uri.TryCreate(string? uriString, UriKind uriKind, out Uri? result) returns false when the input string is null, and it does not throw. Reason: the Microsoft Learn documentation describes TryCreate as returning a bool indicating success and not throwing for invalid URIs, and the reference source implementation checks
if (uriString is null) { return null; }, which causes TryCreate to fail and leaveresultunset/null. [1][2] So for null input, expect: - Return value: false [1] - out parameter: null (uninitialized / null) [1][2] - Exceptions: none [1]Citations:
🏁 Script executed:
Repository: christianhelle/refitter
Length of output: 2992
Guard against null deserialized settings and null elements in
OpenApiPathsLoaddereferencessettingseven when the JSON payload is the valid literal"null":Serializer.Deserialize<RefitGeneratorSettings>("null")returnsnull, soResolveRelativeSpecPaths(settings, ...)throwsNullReferenceExceptioninstead of aJsonException.ResolveRelativeSpecPathsiteratessettings.OpenApiPathswithout handlingnull/blank elements; anullelement will reachPath.IsPathRooted(path)and throwArgumentNullException.Proposed fix
public static RefitGeneratorSettings Load(string json, string baseDirectory) { - var settings = Serializer.Deserialize<RefitGeneratorSettings>(json); + var settings = Serializer.Deserialize<RefitGeneratorSettings>(json) + ?? throw new System.Text.Json.JsonException("The .refitter settings payload must deserialize to an object."); ResolveRelativeSpecPaths(settings, baseDirectory); return settings; }📝 Committable suggestion
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
In .NET, what doPath.IsPathRooted(null),Path.Combine(baseDirectory, ""), andPath.Combine(baseDirectory, null)do?💡 Result:
In.NET, the behaviors for these Path methods are as follows: Path.IsPathRooted(null) Calling Path.IsPathRooted with a null argument throws an ArgumentNullException [1][2][3]. Path.Combine(baseDirectory, "") If the second argument is an empty string (""), Path.Combine returns the baseDirectory (optionally with a directory separator character appended if it is missing) [4][5][6]. It does not throw an exception for an empty string [6]. Path.Combine(baseDirectory, null) Calling Path.Combine with a null argument for any of its path parameters throws an ArgumentNullException [7]. Unlike the empty string, a null value is not treated as a valid path component [6][7].
Citations:
Skip null/whitespace entries in
OpenApiPaths(inResolveRelativeSpecPaths)Path.IsPathRooted(null)throwsArgumentNullException, and empty/whitespace values get normalized intobaseDirectoryviaPath.Combine(baseDirectory, ""), so malformed entries are rewritten instead of being left for validation.Proposed fix
if (settings.OpenApiPaths is { Length: > 0 }) { for (var i = 0; i < settings.OpenApiPaths.Length; i++) { var path = settings.OpenApiPaths[i]; - if (!IsUrl(path) && !Path.IsPathRooted(path)) + if (string.IsNullOrWhiteSpace(path) || IsUrl(path) || Path.IsPathRooted(path)) { - settings.OpenApiPaths[i] = Path.GetFullPath(Path.Combine(baseDirectory, path)); + continue; } + + settings.OpenApiPaths[i] = Path.GetFullPath(Path.Combine(baseDirectory, path)); } }🤖 Prompt for AI Agents
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.