Keep diagnostic locations inside the compilation being analyzed - #192
Open
kairoi-llc wants to merge 1 commit into
Open
kairoi-llc wants to merge 1 commit into
kairoi-llc wants to merge 1 commit into
Conversation
Locations taken from symbols can belong to a different compilation: a registration declared on a [ServiceProviderModule] in a referenced project, or a constructor parameter of an implementation type defined there. Roslyn rejects a diagnostic whose location is outside the compilation being analyzed, on the paths that validate locations — notably .NET Hot Reload / EnC. ContainerGenerator's catch-all then turns the ArgumentException into JAB0001, so no container is generated and `dotnet watch` stops applying changes. Ordinary builds do not validate diagnostic locations, which is why this is invisible outside hot reload. It is reached easily because JAB0014 (NullableServiceRegistered) is Info severity and fires on healthy graphs — any nullable-annotated parameter without a default whose service is registered. Suppression cannot help: ReportDiagnostic validates the location before severity filtering. Adds InCurrentCompilation() and applies it to every symbol-derived location in ServiceProviderBuilder, not only JAB0014 — JAB0002, JAB0013 and JAB0019 are built from the same registrationLocation and would throw identically on a graph that triggers them. Fixes pakrym#191
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
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.
Fixes #191.
Locations taken from symbols can belong to a compilation other than the one being analyzed — a registration declared on a
[ServiceProviderModule]in a referenced project, or a constructor parameter of an implementation type defined there. Roslyn rejects a diagnostic carrying such a location on the paths that validate locations (notably .NET Hot Reload / EnC), andContainerGenerator.Execute's catch-all turns the resultingArgumentExceptioninto JAB0001 — so no container is generated anddotnet watchstops applying changes.This adds a small helper and applies it to every symbol-derived location in
ServiceProviderBuilder:It is deliberately applied to all nine sites rather than only JAB0014:
JAB0002,JAB0013andJAB0019are built from the sameregistrationLocation, andJAB0008/JAB0016/JAB0017fromcontext.RequestLocation, which is derived from a parameter symbol that may also live in another project. They would throw identically on a graph that triggers them; they simply do not fire on a healthy one.A no-op whenever the location is already in the current compilation, so ordinary builds are unaffected.
Verification
Two-project sample per the issue — a
[ServiceProviderModule]in a library with a nullable, no-default constructor parameter whose service is registered, imported by a container in the app project.dotnet watch, first edit →error JAB0001: ... 'JAB0014' has a source location in file '.../Lib/Module.cs', which is not part of the compilation being analyzed, followed byCS1061: 'Container' does not contain a definition for 'GetService'.Also exercised on a larger multi-project Avalonia app (~320 registrations across imported modules in a referenced library, two containers) — a full
dotnet buildis unchanged, and repeated edits underdotnet watchno longer fail generation.I did not add a test:
Jab.Testsbuilds its compilations from single-source fixtures, and reproducing this needs a second compilation referenced by the first so a registration'sSourceTreeis genuinely foreign.A possible follow-up, not included here
NullableServiceRegistered(JAB0014) is the onlyInfo-severity descriptor, and it fires on any nullable-annotated parameter without a default whose service is registered — which is a common and reasonable shape when the parameter is nullable so tests can passnullexplicitly. It is invisible in normal builds, so the location bug went unnoticed until hot reload validated it. Whether that diagnostic should fire so broadly seems worth its own discussion; this PR only stops it from breaking generation.