diff --git a/Documentation/generating-a-screenplay.md b/Documentation/generating-a-screenplay.md index 021813dd3..8369deaf5 100644 --- a/Documentation/generating-a-screenplay.md +++ b/Documentation/generating-a-screenplay.md @@ -35,7 +35,7 @@ The generator is used through the Cratis CLI, which ships separately from Arc: cratis screenplay generate ./MyApp/MyApp.csproj --file MyApp.play ``` -Point it at a project or a solution. Anything the generator could not express is reported on the way out, and a run that reports an error exits non-zero — so a broken build never quietly produces a document that looks fine. +Point it at a project or a solution. Anything the generator could not express is reported on the way out, and a run that reports an error exits non-zero — so a document nothing trustworthy went into never quietly looks fine. What the CLI has to do before it hands the source over is covered in [what the generator expects of its host](#what-the-generator-expects-of-its-host). Because output is reproducible, a CI step can regenerate and fail when the committed `.play` no longer matches the source. @@ -48,7 +48,49 @@ Warning SP0019: The query 'Raw' returns 'IActionResult', which says how the resu transported rather than what it is, so the query was left out (Library.Messaging.Feed) ``` -Diagnostics come in three severities. **Information** means something is worth knowing but the document is complete. **Warning** means something was left out. **Error** means the document should not be trusted at all — the most common cause being source that did not compile. +Diagnostics come in three severities. **Information** means something is worth knowing but the document is complete. **Warning** means something was left out. **Error** means the document should not be trusted at all — either because the generator produced something the language rejects, or because nothing at all was recovered from source the compiler accepted. + +## What the generator expects of its host + +The generator takes a Roslyn `Compilation` and nothing else. It never opens a project file or loads a workspace, which is what lets it be driven from a CLI, from a specification, or from an editor. The other side of that bargain is that **assembling the compilation the way a real build would is the caller's job** — the generator reads what it is handed and cannot tell a missing type from a type that was never written. + +The part hosts get wrong is source generators. `MSBuildWorkspace.GetCompilationAsync()` does not run them, and neither does any loading mode that stops at the compile items the project file lists. An Arc application leans on generation heavily — `[LoggerMessage]` partial classes, strongly-typed resource designer classes, the proxy and metrics generators — so a compilation loaded that way is missing every type those emit, and every reference to one becomes an unresolved-symbol error. This is the common case rather than an edge case. + +A host should run the project's generators before handing the compilation over: + +```csharp +var driver = CSharpGeneratorDriver.Create( + generators: project.AnalyzerReferences + .SelectMany(reference => reference.GetGenerators(LanguageNames.CSharp)) + .Select(GeneratorExtensions.AsSourceGenerator), + parseOptions: (CSharpParseOptions)project.ParseOptions!); + +driver.RunGeneratorsAndUpdateCompilation(compilation, out var generated, out _); +``` + +Then generate from `generated` rather than from `compilation`. + +### What happens when it does not + +Nothing is hidden, and nothing correct is thrown away. A compilation carrying errors is still analyzed, and `SP0024` says what happened — how many errors there were, the first of them, and how many artifacts came through anyway: + +```text +Warning SP0024: The source did not compile - 607 error(s), the first being 'The name +'AccountsMessages' does not exist in the current context'. 341 artifact(s) were recovered +anyway, 341 of them from a declaration no error sits inside, so the document describes those +exactly as the source states them - a missing type named like 'SomethingMessages' or a +designer class usually means the compilation was handed over without the compile items a +build generates (Accounts) +``` + +Its **severity is decided rather than fixed**, because "the source did not compile" covers two outcomes that could not be further apart: + +- **Warning** when at least one artifact was recovered from a declaration no compilation error sits inside. Those artifacts are described exactly as their source states them whatever failed elsewhere, so the run is successful and the document is worth keeping. A compilation missing its generated symbols lands here — the errors sit in code that declares no artifact, and the model is unaffected. +- **Error** when none were, either because nothing was recovered at all or because every declaration something came out of is one the compiler could not make sense of. There is then no part of the document a reader could trust, and a host following the contract exits non-zero. + +A count is used rather than a proportion deliberately: any threshold would make the same recovery pass for a large application and fail for a small one, and zero is the only number that means recovery was *prevented* rather than merely dented. + +Either way the document is written out, so what was recovered can be read. ## The generator checks its own output @@ -65,7 +107,9 @@ and the document is returned as it stands so the line can be read (Library) The document is still written out, so you can open it at the reported line and see what happened. If you hit this, it is a bug worth [reporting](https://github.com/Cratis/Arc/issues) — include the line, and the C# declaration it came from. -Source that did not compile (`SP0024`) suppresses `SP0034`. A model recovered from symbols the compiler never accepted describes an application that does not exist, so a poor document made from it is a consequence of the broken build rather than a second, separate defect. Fix the build and generate again. +Source that did not compile (`SP0024`) suppresses `SP0034` **when it is reported as an error** — a model recovered from symbols the compiler never accepted describes an application that does not exist, so a poor document made from it is a consequence of the broken build rather than a second, separate defect. Fix the build and generate again. + +As a warning it suppresses nothing. That severity says the model stands, and a document built from a model that stands is exactly what the check exists for — suppressing it there would hand back a `.play` the language rejects with nothing wrong reported. ## Screens diff --git a/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_an_error_sits_in_a_partial_the_build_wrote.cs b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_an_error_sits_in_a_partial_the_build_wrote.cs new file mode 100644 index 000000000..e70e26486 --- /dev/null +++ b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_an_error_sits_in_a_partial_the_build_wrote.cs @@ -0,0 +1,57 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Cratis.Arc.Screenplay.Analysis; + +namespace Cratis.Arc.Screenplay.for_ApplicationModelAnalyzer.when_analyzing.source_that_did_not_compile; + +/// +/// A type is written in as many places as it has partial declarations, and the Cratis house style puts one of them in +/// the intermediate folder of the build. An error there is an error inside the artifact just as much as one in the +/// file a person wrote, so every place a type is written counts - reading only the first would call a declaration +/// clean on the strength of the half that happens to be listed first. +/// +public class and_an_error_sits_in_a_partial_the_build_wrote : Specification +{ + const string Written = """ + using System.Threading.Tasks; + using Cratis.Chronicle.Events; + using Cratis.Chronicle.Reactors; + + namespace Library.Onboarding.Registry; + + [EventType] + public record CompanyRegistered(string Name); + + public partial class RegistryNotifier : IReactor + { + public Task CompanyRegistered(CompanyRegistered @event, EventContext context) => Task.CompletedTask; + } + """; + + const string Emitted = """ + namespace Library.Onboarding.Registry; + + public partial class RegistryNotifier + { + public string Describe() => RegistryMessages.Describe; + } + """; + + static readonly (string Path, string Text)[] _sources = + [ + ("Library/Onboarding/Registry/Registry.cs", Written), + ("Library/obj/Debug/net10.0/Generated/Registry.Logging.g.cs", Emitted) + ]; + + ApplicationModelAnalysis _analysis; + + void Establish() => _analysis = Analyzed.Source(_sources); + + ScreenplayDiagnostic Reported => _analysis.Diagnostics.First(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile); + + [Fact] void should_be_analyzing_source_that_really_does_not_compile() => Analyzed.ErrorsIn(_sources).ShouldNotBeEmpty(); + [Fact] void should_report_it_as_a_warning() => Reported.Severity.ShouldEqual(ScreenplayDiagnosticSeverity.Warning); + [Fact] void should_not_count_the_reactor_the_error_is_written_inside() => Reported.Message.Contains("2 artifact(s) were recovered anyway, 1 of them", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_still_return_the_reactor() => _analysis.Slice().Reactors.Single().Name.ShouldEqual("RegistryNotifier"); +} diff --git a/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_every_artifact_it_recovered_carries_an_error.cs b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_every_artifact_it_recovered_carries_an_error.cs new file mode 100644 index 000000000..04996a40a --- /dev/null +++ b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_every_artifact_it_recovered_carries_an_error.cs @@ -0,0 +1,40 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Cratis.Arc.Screenplay.Analysis; + +namespace Cratis.Arc.Screenplay.for_ApplicationModelAnalyzer.when_analyzing.source_that_did_not_compile; + +/// +/// Something was recovered, but every declaration it came out of is one the compiler could not make sense of - so +/// what the document says about each of them is read from source that never held together. That is the other way +/// recovery is genuinely prevented, and it is an error for the same reason as recovering nothing at all: there is +/// no part of the document a reader could trust. +/// +public class and_every_artifact_it_recovered_carries_an_error : Specification +{ + const string Source = """ + using Cratis.Arc.Commands.ModelBound; + + namespace Library.Authors.Registration; + + [Command] + public record RegisterAuthor(string Name) + { + public void Handle() => ThisDoesNotExist(); + } + """; + + ApplicationModelAnalysis _analysis; + + void Establish() => _analysis = Analyzed.Source(Source); + + ScreenplayDiagnostic Reported => _analysis.Diagnostics.First(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile); + + [Fact] void should_be_analyzing_source_that_really_does_not_compile() => Analyzed.ErrorsIn((Analyzed.SlicePath, Source)).ShouldNotBeEmpty(); + [Fact] void should_report_it_as_an_error() => Reported.Severity.ShouldEqual(ScreenplayDiagnosticSeverity.Error); + [Fact] void should_say_how_much_it_recovered() => Reported.Message.Contains("1 artifact(s) were recovered", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_say_the_errors_sit_inside_what_it_recovered() => Reported.Message.Contains("every declaration they were read from is one an error sits inside", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_say_the_document_cannot_be_relied_on() => Reported.Message.Contains("describes the application reliably", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_still_return_whatever_it_recovered() => _analysis.Slice().Commands.Single().Name.ShouldEqual("RegisterAuthor"); +} diff --git a/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile.cs b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_nothing_at_all_was_recovered.cs similarity index 53% rename from Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile.cs rename to Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_nothing_at_all_was_recovered.cs index b5cb660f1..bf521480c 100644 --- a/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile.cs +++ b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_nothing_at_all_was_recovered.cs @@ -3,29 +3,22 @@ using Cratis.Arc.Screenplay.Analysis; -namespace Cratis.Arc.Screenplay.for_ApplicationModelAnalyzer.when_analyzing; +namespace Cratis.Arc.Screenplay.for_ApplicationModelAnalyzer.when_analyzing.source_that_did_not_compile; /// -/// Source that does not build still yields symbols, and analyzing them produces a document that looks like an -/// answer while describing an application that does not exist. A nearly empty document handed back with nothing -/// wrong reported is the one outcome nobody can act on, so this is an error - which is what makes a host exit non -/// zero - while whatever was recovered is still returned for whoever wants to look at it. +/// A build that produced nothing an artifact could be read out of is the case the claim was always true for - a +/// document describing an empty application handed back with nothing wrong reported is the one outcome nobody can +/// act on, so this is an error and a host exits non zero on it. It is also the case that must not be mistaken for +/// an application which simply declares no artifact yet, so "declares nothing" stays suppressed. /// -public class source_that_did_not_compile : Specification +public class and_nothing_at_all_was_recovered : Specification { const string Source = """ - using Cratis.Arc.Commands.ModelBound; - using Cratis.Chronicle.Events; + namespace Library.Plumbing; - namespace Library.Authors.Registration; - - [EventType] - public record AuthorRegistered(string Name); - - [Command] - public record RegisterAuthor(string Name) + public class NotAnArtifact { - public AuthorRegistered Handle() => new(ThisDoesNotExist); + public string Name() => ThisDoesNotExist; } """; @@ -35,11 +28,13 @@ public record RegisterAuthor(string Name) ScreenplayDiagnostic Reported => _analysis.Diagnostics.First(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile); - [Fact] void should_be_analyzing_source_that_really_does_not_compile() => Analyzed.ErrorsIn(("Library/Feature/Slice/Slice.cs", Source)).ShouldNotBeEmpty(); + [Fact] void should_be_analyzing_source_that_really_does_not_compile() => Analyzed.ErrorsIn((Analyzed.SlicePath, Source)).ShouldNotBeEmpty(); [Fact] void should_report_that_the_source_did_not_compile() => _analysis.Diagnostics.Select(_ => _.Code).ShouldContain(ScreenplayDiagnosticCodes.SourceDidNotCompile); [Fact] void should_report_it_as_an_error() => Reported.Severity.ShouldEqual(ScreenplayDiagnosticSeverity.Error); + [Fact] void should_say_nothing_at_all_came_out_of_it() => Reported.Message.Contains("Nothing at all was recovered from it", StringComparison.Ordinal).ShouldBeTrue(); [Fact] void should_say_the_document_cannot_be_relied_on() => Reported.Message.Contains("describes the application reliably", StringComparison.Ordinal).ShouldBeTrue(); [Fact] void should_quote_the_first_thing_the_compiler_said() => Reported.Message.Contains("ThisDoesNotExist", StringComparison.Ordinal).ShouldBeTrue(); - [Fact] void should_still_return_whatever_it_recovered() => _analysis.Slice().Commands.Single().Name.ShouldEqual("RegisterAuthor"); + [Fact] void should_not_also_say_the_application_declares_nothing() => _analysis.Diagnostics.Select(_ => _.Code).ShouldNotContain(ScreenplayDiagnosticCodes.AnalysisUnavailable); + [Fact] void should_recover_no_slice() => _analysis.Model.Slices.ShouldBeEmpty(); [Fact] void should_not_throw_instead() => _analysis.Model.ShouldNotBeNull(); } diff --git a/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_only_some_of_what_it_recovered_carries_an_error.cs b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_only_some_of_what_it_recovered_carries_an_error.cs new file mode 100644 index 000000000..5054f2cbb --- /dev/null +++ b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_only_some_of_what_it_recovered_carries_an_error.cs @@ -0,0 +1,44 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Cratis.Arc.Screenplay.Analysis; + +namespace Cratis.Arc.Screenplay.for_ApplicationModelAnalyzer.when_analyzing.source_that_did_not_compile; + +/// +/// This is the boundary between the two severities. One artifact was read from a declaration an error sits inside +/// and one was not, and the one that was not is described exactly as its source states it - so "nothing recovered +/// describes the application reliably" is already false with a single clean declaration. The severity therefore +/// turns on whether any survived rather than on whether any was hit, and the count says which is which. +/// +public class and_only_some_of_what_it_recovered_carries_an_error : Specification +{ + const string Source = """ + using Cratis.Arc.Commands.ModelBound; + using Cratis.Chronicle.Events; + + namespace Library.Authors.Registration; + + [EventType] + public record AuthorRegistered(string Name); + + [Command] + public record RegisterAuthor(string Name) + { + public AuthorRegistered Handle() => new(ThisDoesNotExist); + } + """; + + ApplicationModelAnalysis _analysis; + + void Establish() => _analysis = Analyzed.Source(Source); + + ScreenplayDiagnostic Reported => _analysis.Diagnostics.First(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile); + + [Fact] void should_be_analyzing_source_that_really_does_not_compile() => Analyzed.ErrorsIn((Analyzed.SlicePath, Source)).ShouldNotBeEmpty(); + [Fact] void should_report_it_as_a_warning() => Reported.Severity.ShouldEqual(ScreenplayDiagnosticSeverity.Warning); + [Fact] void should_report_nothing_at_all_as_an_error() => _analysis.Diagnostics.Any(_ => _.Severity == ScreenplayDiagnosticSeverity.Error).ShouldBeFalse(); + [Fact] void should_count_only_the_one_no_error_sits_inside() => Reported.Message.Contains("2 artifact(s) were recovered anyway, 1 of them", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_quote_the_first_thing_the_compiler_said() => Reported.Message.Contains("ThisDoesNotExist", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_still_return_whatever_it_recovered() => _analysis.Slice().Commands.Single().Name.ShouldEqual("RegisterAuthor"); +} diff --git a/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_the_errors_sit_outside_what_it_recovered.cs b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_the_errors_sit_outside_what_it_recovered.cs new file mode 100644 index 000000000..83e8793f2 --- /dev/null +++ b/Source/DotNET/Screenplay.Specs/for_ApplicationModelAnalyzer/when_analyzing/source_that_did_not_compile/and_the_errors_sit_outside_what_it_recovered.cs @@ -0,0 +1,65 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Cratis.Arc.Screenplay.Analysis; + +namespace Cratis.Arc.Screenplay.for_ApplicationModelAnalyzer.when_analyzing.source_that_did_not_compile; + +/// +/// This is the case that made the claim a lie. A host handing over a compilation assembled without the compile items +/// a build generates leaves every reference to a strongly typed resource class unresolved, and the errors then sit in +/// source that declares no artifact at all while every command and event is read exactly as written. Calling that an +/// error says something untrue about a document that is entirely correct and makes the host throw it away, so it is a +/// warning that states both facts - what failed, and how much came through anyway. +/// +public class and_the_errors_sit_outside_what_it_recovered : Specification +{ + const string Slice = """ + using Cratis.Arc.Commands.ModelBound; + using Cratis.Chronicle.Events; + + namespace Library.Authors.Registration; + + [EventType] + public record AuthorRegistered(string Name); + + [Command] + public record RegisterAuthor(string Name) + { + public AuthorRegistered Handle() => new(Name); + } + """; + + const string Wording = """ + namespace Library.Authors; + + public static class Wording + { + public static string NameIsRequired() => AuthorsMessages.NameIsRequired; + } + """; + + static readonly (string Path, string Text)[] _sources = + [ + ("Library/Authors/Registration/Registration.cs", Slice), + ("Library/Authors/Wording.cs", Wording) + ]; + + ApplicationModelAnalysis _analysis; + + void Establish() => _analysis = Analyzed.Source(_sources); + + ScreenplayDiagnostic Reported => _analysis.Diagnostics.First(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile); + + [Fact] void should_be_analyzing_source_that_really_does_not_compile() => Analyzed.ErrorsIn(_sources).ShouldNotBeEmpty(); + [Fact] void should_report_that_the_source_did_not_compile() => _analysis.Diagnostics.Select(_ => _.Code).ShouldContain(ScreenplayDiagnosticCodes.SourceDidNotCompile); + [Fact] void should_report_it_as_a_warning() => Reported.Severity.ShouldEqual(ScreenplayDiagnosticSeverity.Warning); + [Fact] void should_report_nothing_at_all_as_an_error() => _analysis.Diagnostics.Any(_ => _.Severity == ScreenplayDiagnosticSeverity.Error).ShouldBeFalse(); + [Fact] void should_say_how_many_errors_there_were() => Reported.Message.Contains("1 error(s)", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_quote_the_first_thing_the_compiler_said() => Reported.Message.Contains("AuthorsMessages", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_say_how_much_it_recovered_anyway() => Reported.Message.Contains("2 artifact(s) were recovered anyway, 2 of them", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_hint_at_what_the_host_left_out() => Reported.Message.Contains("without the compile items a build generates", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_not_claim_nothing_describes_the_application_reliably() => Reported.Message.Contains("describes the application reliably", StringComparison.Ordinal).ShouldBeFalse(); + [Fact] void should_recover_the_command() => _analysis.Slice().Commands.Single().Name.ShouldEqual("RegisterAuthor"); + [Fact] void should_recover_the_event() => _analysis.Slice().Events.Single().Name.ShouldEqual("AuthorRegistered"); +} diff --git a/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/RecoveringSource.cs b/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/RecoveringSource.cs new file mode 100644 index 000000000..ba1949517 --- /dev/null +++ b/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/RecoveringSource.cs @@ -0,0 +1,56 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +namespace Cratis.Arc.Screenplay.for_ScreenplayGenerator.when_generating.and_the_source_it_read_did_not_compile; + +/// +/// Holds source that does not compile and yet gives up everything it declares, which is what a host handing over a +/// compilation assembled without the compile items a build generates really produces. +/// +/// +/// The unresolved name sits in a helper class that declares no artifact, so the errors are real while the command and +/// the event are read from declarations the compiler accepted - the shape the whole recalibration turns on. +/// +public static class RecoveringSource +{ + /// + /// The slice, which compiles on its own. + /// + public const string Slice = """ + using Cratis.Arc.Commands.ModelBound; + using Cratis.Chronicle.Events; + + namespace Library.Authors.Registration; + + [EventType] + public record AuthorRegistered(string Name); + + [Command] + public record RegisterAuthor(string Name) + { + public AuthorRegistered Handle() => new(Name); + } + """; + + /// + /// A helper reaching for the strongly typed resource class a build would have generated. + /// + public const string Wording = """ + namespace Library.Authors; + + public static class Wording + { + public static string NameIsRequired() => AuthorsMessages.NameIsRequired; + } + """; + + /// + /// Gets the source files, keyed by the path each one is compiled as. + /// + /// The source files. + public static (string Path, string Text)[] Files() => + [ + ("Library/Authors/Registration/Registration.cs", Slice), + ("Library/Authors/Wording.cs", Wording) + ]; +} diff --git a/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile.cs b/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/and_nothing_was_recovered_from_it.cs similarity index 67% rename from Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile.cs rename to Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/and_nothing_was_recovered_from_it.cs index 19845d8bc..20ae761eb 100644 --- a/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile.cs +++ b/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/and_nothing_was_recovered_from_it.cs @@ -5,15 +5,15 @@ using Cratis.Screenplay; using Microsoft.CodeAnalysis; -namespace Cratis.Arc.Screenplay.for_ScreenplayGenerator.when_generating; +namespace Cratis.Arc.Screenplay.for_ScreenplayGenerator.when_generating.and_the_source_it_read_did_not_compile; /// -/// Source the compiler never accepted still yields symbols, and a model recovered from them describes an application -/// that does not exist - so a document made from it being poor is the consequence already reported rather than a -/// second defect. Saying both would send whoever reads the diagnostics hunting for a bug in the generator when the -/// thing to fix is the build, which is why the same suppression the empty document already gets applies here. +/// Source the compiler never accepted that yielded no artifact leaves a model describing an application that does not +/// exist - so a document made from it being poor is the consequence already reported rather than a second defect. +/// Saying both would send whoever reads the diagnostics hunting for a bug in the generator when the thing to fix is +/// the build, which is why the same suppression the empty document already gets applies here. /// -public class and_the_source_it_read_did_not_compile : given.a_document_the_language_rejects +public class and_nothing_was_recovered_from_it : given.a_document_the_language_rejects { const string Source = """ namespace Library.Authors.Registration; @@ -36,9 +36,12 @@ void Establish() void Because() => _result = _generator.Generate(_compilation, new ScreenplayOptions()); + ScreenplayDiagnostic Reported => _result.Diagnostics.First(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile); + [Fact] void should_be_generating_from_source_that_really_does_not_compile() => Analyzed.ErrorsIn((Analyzed.SlicePath, Source)).ShouldNotBeEmpty(); [Fact] void should_be_printing_a_document_the_language_really_rejects() => new ScreenplayCompiler().Compile(Rejected).Success.ShouldBeFalse(); [Fact] void should_report_that_the_source_did_not_compile() => _result.Diagnostics.Select(_ => _.Code).ShouldContain(ScreenplayDiagnosticCodes.SourceDidNotCompile); + [Fact] void should_report_it_as_an_error() => Reported.Severity.ShouldEqual(ScreenplayDiagnosticSeverity.Error); [Fact] void should_not_report_the_document_on_top_of_it() => _result.Diagnostics.Select(_ => _.Code).ShouldNotContain(ScreenplayDiagnosticCodes.DocumentDidNotCompile); [Fact] void should_still_return_the_document() => _result.Source.ShouldEqual(Rejected); [Fact] void should_not_be_successful() => _result.IsSuccess.ShouldBeFalse(); diff --git a/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/and_the_document_it_printed_is_rejected_too.cs b/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/and_the_document_it_printed_is_rejected_too.cs new file mode 100644 index 000000000..afacb0ffd --- /dev/null +++ b/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/and_the_document_it_printed_is_rejected_too.cs @@ -0,0 +1,38 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Cratis.Arc.Screenplay.Analysis; +using Cratis.Screenplay; +using Microsoft.CodeAnalysis; + +namespace Cratis.Arc.Screenplay.for_ScreenplayGenerator.when_generating.and_the_source_it_read_did_not_compile; + +/// +/// The suppression of the document check follows the severity rather than the code. Source that did not compile but +/// gave up everything it declares says the model stands, and a document built from a model that stands is exactly +/// what the check exists for - suppressing it here would hand back a document the language rejects with nothing +/// wrong reported, which is the one outcome nobody can act on. +/// +public class and_the_document_it_printed_is_rejected_too : given.a_document_the_language_rejects +{ + Compilation _compilation; + ScreenplayGenerator _generator; + ScreenplayGenerationResult _result; + + void Establish() + { + _compilation = Analyzed.Compile(RecoveringSource.Files()); + _generator = new(new ApplicationModelAnalyzer(), _emitter); + } + + void Because() => _result = _generator.Generate(_compilation, new ScreenplayOptions()); + + ScreenplayDiagnostic Reported => _result.Diagnostics.First(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile); + + [Fact] void should_be_generating_from_source_that_really_does_not_compile() => Analyzed.ErrorsIn(RecoveringSource.Files()).ShouldNotBeEmpty(); + [Fact] void should_be_printing_a_document_the_language_really_rejects() => new ScreenplayCompiler().Compile(Rejected).Success.ShouldBeFalse(); + [Fact] void should_report_the_source_it_read_only_as_a_warning() => Reported.Severity.ShouldEqual(ScreenplayDiagnosticSeverity.Warning); + [Fact] void should_report_the_document_on_top_of_it() => _result.Diagnostics.Select(_ => _.Code).ShouldContain(ScreenplayDiagnosticCodes.DocumentDidNotCompile); + [Fact] void should_still_return_the_document() => _result.Source.ShouldEqual(Rejected); + [Fact] void should_not_be_successful() => _result.IsSuccess.ShouldBeFalse(); +} diff --git a/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/and_what_it_recovered_still_stands.cs b/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/and_what_it_recovered_still_stands.cs new file mode 100644 index 000000000..952c6b3ba --- /dev/null +++ b/Source/DotNET/Screenplay.Specs/for_ScreenplayGenerator/when_generating/and_the_source_it_read_did_not_compile/and_what_it_recovered_still_stands.cs @@ -0,0 +1,44 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Cratis.Screenplay; +using Microsoft.CodeAnalysis; + +namespace Cratis.Arc.Screenplay.for_ScreenplayGenerator.when_generating.and_the_source_it_read_did_not_compile; + +/// +/// This is the point of the whole thing. A host that follows the documented contract discards the document on +/// anything reported as an error, so a compilation that lost only its generated symbols used to cost the caller a +/// complete and correct document. The errors are still reported - nothing is hidden - but the result is successful, +/// because what was recovered is described exactly as the source states it. +/// +public class and_what_it_recovered_still_stands : Specification +{ + Compilation _compilation; + ScreenplayGenerator _generator; + ScreenplayGenerationResult _result; + CompilationResult _compiled; + + void Establish() + { + _compilation = Analyzed.Compile(RecoveringSource.Files()); + _generator = new(); + } + + void Because() + { + _result = _generator.Generate(_compilation, new ScreenplayOptions()); + _compiled = new ScreenplayCompiler().Compile(_result.Source); + } + + ScreenplayDiagnostic Reported => _result.Diagnostics.First(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile); + + [Fact] void should_be_generating_from_source_that_really_does_not_compile() => Analyzed.ErrorsIn(RecoveringSource.Files()).ShouldNotBeEmpty(); + [Fact] void should_report_that_the_source_did_not_compile() => _result.Diagnostics.Select(_ => _.Code).ShouldContain(ScreenplayDiagnosticCodes.SourceDidNotCompile); + [Fact] void should_report_it_as_a_warning() => Reported.Severity.ShouldEqual(ScreenplayDiagnosticSeverity.Warning); + [Fact] void should_be_successful() => _result.IsSuccess.ShouldBeTrue(); + [Fact] void should_report_nothing_at_all_as_an_error() => _result.Diagnostics.Any(_ => _.Severity == ScreenplayDiagnosticSeverity.Error).ShouldBeFalse(); + [Fact] void should_describe_the_command_the_source_declares() => _result.Source.Contains("command RegisterAuthor", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_describe_the_event_the_source_declares() => _result.Source.Contains("event AuthorRegistered", StringComparison.Ordinal).ShouldBeTrue(); + [Fact] void should_produce_a_document_that_compiles() => _compiled.Success.ShouldBeTrue(); +} diff --git a/Source/DotNET/Screenplay/Analysis/ApplicationModelAnalyzer.cs b/Source/DotNET/Screenplay/Analysis/ApplicationModelAnalyzer.cs index d33d66f6a..ecf4b44b1 100644 --- a/Source/DotNET/Screenplay/Analysis/ApplicationModelAnalyzer.cs +++ b/Source/DotNET/Screenplay/Analysis/ApplicationModelAnalyzer.cs @@ -38,7 +38,6 @@ public ApplicationModelAnalyzer() public ApplicationModelAnalysis Analyze(Compilation compilation, ScreenplayOptions options) { var diagnostics = new ScreenplayDiagnostics(); - var failedToCompile = ReportCompilationErrors(compilation, diagnostics); var catalog = ArtifactCatalog.From(compilation); var readers = ArtifactReaders.For(compilation, catalog, diagnostics); var screens = new ScreenReader( @@ -47,7 +46,8 @@ public ApplicationModelAnalysis Analyze(Compilation compilation, ScreenplayOptio new(diagnostics), new(userInterfaceFiles, diagnostics)); - var reader = new SliceReader(readers, diagnostics, screens); + var recovered = new RecoveredArtifacts(); + var reader = new SliceReader(readers, diagnostics, screens, recovered); var slices = catalog.Namespaces .Select(@namespace => reader.Read(@namespace, catalog.In(@namespace))) @@ -60,6 +60,13 @@ public ApplicationModelAnalysis Analyze(Compilation compilation, ScreenplayOptio var imports = ExternalEvents.Resolve(compilation, slices, diagnostics); ReportNamespacesWithoutStructure(slices, diagnostics, options.SegmentsToSkip ?? 0); + // How serious source that did not compile is depends on how much survived it, so it is reported once the + // slices are in rather than on the way in. Source that did not compile still suppresses "declares nothing" - + // an empty document from a broken build is the broken build, not an application with nothing in it - and + // when it is reported as a warning the suppression can never apply, because a warning is only ever reached + // when something was recovered and something recovered is a slice. + var failedToCompile = CompilationErrors.Report(compilation, recovered, diagnostics); + if (slices.Count == 0 && !failedToCompile) { diagnostics.Information( @@ -81,35 +88,6 @@ public ApplicationModelAnalysis Analyze(Compilation compilation, ScreenplayOptio diagnostics.All); } - /// - /// Reports source that did not compile, which nothing recovered from it can be relied on past. - /// - /// The compilation to check. - /// The diagnostics to report to. - /// True when the source did not compile. - /// - /// A compilation that does not build still yields symbols, and analyzing them produces a document that looks - /// like an answer while describing an application that does not exist. Reporting it as an error rather than a - /// warning is what makes a host exit non zero, because a nearly empty document and a success code is the one - /// outcome nobody can act on. The model is still returned - what was recovered is worth seeing, as long as - /// nobody is told it is trustworthy. - /// - static bool ReportCompilationErrors(Compilation compilation, ScreenplayDiagnostics diagnostics) - { - var errors = compilation.GetDiagnostics().Where(_ => _.Severity == DiagnosticSeverity.Error).ToList(); - if (errors.Count == 0) - { - return false; - } - - diagnostics.Error( - ScreenplayDiagnosticCodes.SourceDidNotCompile, - $"The source did not compile - {errors.Count} error(s), the first being '{errors[0].GetMessage(System.Globalization.CultureInfo.InvariantCulture)}'. Nothing recovered from it describes the application reliably", - compilation.AssemblyName); - - return true; - } - /// /// Gets everything within a slice that requires something of the caller. /// diff --git a/Source/DotNET/Screenplay/Analysis/ArtifactDeclaration.cs b/Source/DotNET/Screenplay/Analysis/ArtifactDeclaration.cs new file mode 100644 index 000000000..81dfab5f7 --- /dev/null +++ b/Source/DotNET/Screenplay/Analysis/ArtifactDeclaration.cs @@ -0,0 +1,53 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Text; + +namespace Cratis.Arc.Screenplay.Analysis; + +/// +/// Holds where the declaration that artifacts were recovered from is written. +/// +/// +/// A type is written in as many places as it has partial declarations, and a source generator contributing one of +/// them is exactly the case this exists to tell apart, so every place it is written is held rather than the first. +/// +public class ArtifactDeclaration +{ + readonly (SyntaxTree Tree, TextSpan Span)[] _written; + + ArtifactDeclaration(int artifacts, (SyntaxTree Tree, TextSpan Span)[] written) + { + Artifacts = artifacts; + _written = written; + } + + /// + /// Gets the number of artifacts recovered from the declaration. + /// + public int Artifacts { get; } + + /// + /// Holds where the type artifacts were recovered from is written. + /// + /// The type they were recovered from. + /// The number of artifacts recovered from it. + /// The . + public static ArtifactDeclaration For(INamedTypeSymbol type, int artifacts) => + new(artifacts, [.. type.DeclaringSyntaxReferences.Select(_ => (_.SyntaxTree, _.Span))]); + + /// + /// Answers whether something the compiler reported sits within the declaration. + /// + /// The diagnostic the compiler reported. + /// True when it sits within the declaration. + /// + /// An error somewhere else in the compilation says nothing about what was read here, while one written inside + /// the declaration itself is the compiler saying it could not make sense of the very source the artifact was + /// recovered from. Telling the two apart is the whole reason the spans are kept. + /// + public bool Encloses(Diagnostic diagnostic) => + diagnostic.Location.SourceTree is { } tree && + _written.Any(_ => ReferenceEquals(_.Tree, tree) && _.Span.Contains(diagnostic.Location.SourceSpan)); +} diff --git a/Source/DotNET/Screenplay/Analysis/CompilationErrors.cs b/Source/DotNET/Screenplay/Analysis/CompilationErrors.cs new file mode 100644 index 000000000..6cdf80f46 --- /dev/null +++ b/Source/DotNET/Screenplay/Analysis/CompilationErrors.cs @@ -0,0 +1,93 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using System.Globalization; +using Microsoft.CodeAnalysis; + +namespace Cratis.Arc.Screenplay.Analysis; + +/// +/// Reports source that did not compile, at the severity what was recovered from it earns. +/// +/// +/// A compilation that does not build still yields symbols, and analyzing them can produce a document that looks like +/// an answer while describing an application that does not exist - which is why this is reported at all. What it is +/// not is a single outcome. A host that hands over a compilation assembled without the compile items a build +/// generates leaves every reference to a strongly typed resource class unresolved, and hundreds of errors then sit +/// in source that declares no artifact at all while every command, event and reactor is read exactly as written. +/// Calling that an error says something untrue about thousands of correct lines and makes the host discard them. +/// +/// So the severity follows one number: how many artifacts were recovered from a declaration no compilation error +/// sits inside. When that number is zero - because nothing was recovered, or because every declaration something was +/// recovered from is one the compiler could not make sense of - nothing in the document is worth trusting and this is +/// an error, which is what makes a host exit non zero. Otherwise it is a warning stating how much was recovered +/// anyway, because the artifacts read from source the compiler accepted are described exactly as that source states +/// them and a document holding them is worth having. +/// +/// +/// A count is used rather than a proportion deliberately. Any threshold - a tenth, a half - would make the same +/// recovery pass for a large application and fail for a small one, and no number is defensible. Zero is the only one +/// that means recovery was prevented rather than merely dented. +/// +/// +public static class CompilationErrors +{ + /// + /// Reports the errors a compilation carries, if it carries any. + /// + /// The compilation that was analyzed. + /// What was recovered from it, and where each of it was written. + /// The diagnostics to report to. + /// True when the source did not compile, whatever was recovered from it. + public static bool Report(Compilation compilation, RecoveredArtifacts recovered, ScreenplayDiagnostics diagnostics) + { + var errors = compilation.GetDiagnostics().Where(_ => _.Severity == DiagnosticSeverity.Error).ToList(); + if (errors.Count == 0) + { + return false; + } + + var first = errors[0].GetMessage(CultureInfo.InvariantCulture); + var accepted = recovered.RecoveredFromAcceptedSource(errors); + + if (accepted == 0) + { + diagnostics.Error( + ScreenplayDiagnosticCodes.SourceDidNotCompile, + PreventedRecovery(errors.Count, first, recovered.Count), + compilation.AssemblyName); + } + else + { + diagnostics.Warning( + ScreenplayDiagnosticCodes.SourceDidNotCompile, + SurvivedRecovery(errors.Count, first, recovered.Count, accepted), + compilation.AssemblyName); + } + + return true; + } + + /// + /// Says that the errors left nothing behind worth trusting. + /// + /// The number of errors the compiler reported. + /// The first thing the compiler said. + /// The number of artifacts recovered in all. + /// The message. + static string PreventedRecovery(int errors, string first, int recovered) => + recovered == 0 + ? $"The source did not compile - {errors} error(s), the first being '{first}'. Nothing at all was recovered from it, so nothing in the document describes the application reliably" + : $"The source did not compile - {errors} error(s), the first being '{first}'. {recovered} artifact(s) were recovered, and every declaration they were read from is one an error sits inside, so nothing recovered describes the application reliably"; + + /// + /// Says how much came through the errors intact. + /// + /// The number of errors the compiler reported. + /// The first thing the compiler said. + /// The number of artifacts recovered in all. + /// The number of them read from a declaration no error sits inside. + /// The message. + static string SurvivedRecovery(int errors, string first, int recovered, int accepted) => + $"The source did not compile - {errors} error(s), the first being '{first}'. {recovered} artifact(s) were recovered anyway, {accepted} of them from a declaration no error sits inside, so the document describes those exactly as the source states them - a missing type named like 'SomethingMessages' or a designer class usually means the compilation was handed over without the compile items a build generates"; +} diff --git a/Source/DotNET/Screenplay/Analysis/RecoveredArtifacts.cs b/Source/DotNET/Screenplay/Analysis/RecoveredArtifacts.cs new file mode 100644 index 000000000..03edc2d08 --- /dev/null +++ b/Source/DotNET/Screenplay/Analysis/RecoveredArtifacts.cs @@ -0,0 +1,53 @@ +// Copyright (c) Cratis. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.CodeAnalysis; + +namespace Cratis.Arc.Screenplay.Analysis; + +/// +/// Counts what analysis recovered from a compilation, and remembers where each of it was written. +/// +/// +/// "Nothing recovered from it describes the application reliably" is a claim about a compilation that did not +/// compile, and for most of them it is false - a build handed over without the compile items it generates leaves +/// hundreds of errors behind while every artifact is read exactly as written. This is what turns that claim into +/// one that can be checked: how much was recovered, and how much of it came from source the compiler accepted. +/// +public class RecoveredArtifacts +{ + readonly List _declarations = []; + + /// + /// Gets the number of artifacts recovered in all. + /// + public int Count => _declarations.Sum(_ => _.Artifacts); + + /// + /// Holds what one type contributed, ignoring a type that contributed nothing. + /// + /// The type that was read. + /// The number of artifacts recovered from it. + public void Declare(INamedTypeSymbol type, int artifacts) + { + if (artifacts <= 0) + { + return; + } + + _declarations.Add(ArtifactDeclaration.For(type, artifacts)); + } + + /// + /// Counts the artifacts recovered from a declaration that none of the compilation errors sit inside. + /// + /// The errors the compiler reported. + /// The number of artifacts. + /// + /// This is the number that decides how serious a compilation that did not compile is. An artifact read from a + /// declaration the compiler accepted is described exactly as the source states it, whatever went wrong + /// elsewhere, so as long as there is one of them the document is not worthless. + /// + public int RecoveredFromAcceptedSource(IReadOnlyList errors) => + _declarations.Where(declaration => !errors.Any(declaration.Encloses)).Sum(_ => _.Artifacts); +} diff --git a/Source/DotNET/Screenplay/Analysis/Slices/SliceArtifactReader.cs b/Source/DotNET/Screenplay/Analysis/Slices/SliceArtifactReader.cs index 0b17f0409..73b6dc1ec 100644 --- a/Source/DotNET/Screenplay/Analysis/Slices/SliceArtifactReader.cs +++ b/Source/DotNET/Screenplay/Analysis/Slices/SliceArtifactReader.cs @@ -19,11 +19,12 @@ namespace Cratis.Arc.Screenplay.Analysis.Slices; /// /// The reading each kind of artifact. /// The anything unmappable is reported to. +/// The holding what each declaration yielded. /// /// A type can be more than one thing at once - a read model with queries is also the declaration of a projection - /// so every recognizer is asked in turn rather than the first match winning. /// -public class SliceArtifactReader(ArtifactReaders readers, ScreenplayDiagnostics diagnostics) +public class SliceArtifactReader(ArtifactReaders readers, ScreenplayDiagnostics diagnostics, RecoveredArtifacts recovered) { /// /// Reads everything one type contributes. @@ -31,7 +32,26 @@ public class SliceArtifactReader(ArtifactReaders readers, ScreenplayDiagnostics /// The type to read. /// The namespace the slice lives in. /// The content collected so far. + /// + /// What the type yielded is taken as the difference it made to the content rather than recorded by each + /// recognizer, so a recognizer added later is tied to its declaration without anyone remembering to say so. + /// public void Read(INamedTypeSymbol type, string @namespace, SliceContents content) + { + var before = content.Count; + + ReadInto(type, @namespace, content); + + recovered.Declare(type, content.Count - before); + } + + /// + /// Asks every recognizer what the type is. + /// + /// The type to read. + /// The namespace the slice lives in. + /// The content collected so far. + void ReadInto(INamedTypeSymbol type, string @namespace, SliceContents content) { if (EventReader.IsEvent(type)) { diff --git a/Source/DotNET/Screenplay/Analysis/Slices/SliceContents.cs b/Source/DotNET/Screenplay/Analysis/Slices/SliceContents.cs index 4360669d3..f27e22e9c 100644 --- a/Source/DotNET/Screenplay/Analysis/Slices/SliceContents.cs +++ b/Source/DotNET/Screenplay/Analysis/Slices/SliceContents.cs @@ -49,14 +49,23 @@ public class SliceContents /// public bool HasAggregateRoot { get; set; } + /// + /// Gets the number of artifacts collected so far. + /// + /// + /// What one type contributed is the difference between this before it was read and after, which is how a + /// declaration is tied to what came out of it without every recognizer having to say so itself. + /// + public int Count => + Commands.Count + + Events.Count + + Queries.Count + + Reactors.Count + + Constraints.Count + + (Projection is null ? 0 : 1); + /// /// Gets a value indicating whether the slice declares nothing at all. /// - public bool IsEmpty => - Commands.Count == 0 && - Events.Count == 0 && - Queries.Count == 0 && - Reactors.Count == 0 && - Constraints.Count == 0 && - Projection is null; + public bool IsEmpty => Count == 0; } diff --git a/Source/DotNET/Screenplay/Analysis/Slices/SliceReader.cs b/Source/DotNET/Screenplay/Analysis/Slices/SliceReader.cs index 12a78e7fe..e8c164c01 100644 --- a/Source/DotNET/Screenplay/Analysis/Slices/SliceReader.cs +++ b/Source/DotNET/Screenplay/Analysis/Slices/SliceReader.cs @@ -13,14 +13,19 @@ namespace Cratis.Arc.Screenplay.Analysis.Slices; /// The reading each kind of artifact. /// The anything unmappable is reported to. /// The reading what the slice ends in. +/// The holding what each declaration yielded. /// /// A namespace is the unit a slice is recovered from, because that is what the vertical slice convention makes it - /// everything belonging to one behavior lives together, and nothing else does. The same convention puts the file /// realizing a screen in that folder too, which is why screens are read here rather than discovered separately. /// -public class SliceReader(ArtifactReaders readers, ScreenplayDiagnostics diagnostics, ScreenReader screens) +public class SliceReader( + ArtifactReaders readers, + ScreenplayDiagnostics diagnostics, + ScreenReader screens, + RecoveredArtifacts recovered) { - readonly SliceArtifactReader _artifacts = new(readers, diagnostics); + readonly SliceArtifactReader _artifacts = new(readers, diagnostics, recovered); /// /// Reads the slice a namespace declares. diff --git a/Source/DotNET/Screenplay/ScreenplayDiagnosticCodes.cs b/Source/DotNET/Screenplay/ScreenplayDiagnosticCodes.cs index f206ea2ab..06286a787 100644 --- a/Source/DotNET/Screenplay/ScreenplayDiagnosticCodes.cs +++ b/Source/DotNET/Screenplay/ScreenplayDiagnosticCodes.cs @@ -133,8 +133,31 @@ public static class ScreenplayDiagnosticCodes public const string NamespaceWithoutStructure = "SP0023"; /// - /// The source did not compile, so nothing recovered from it can be relied on. + /// The source did not compile, and how far what was recovered from it can be relied on is what the severity says. /// + /// + /// This is the one code whose severity is decided rather than fixed, because "the source did not compile" covers + /// two outcomes that could not be further apart. A host handing over a compilation assembled without the compile + /// items a build generates leaves every reference to a generated type unresolved - hundreds of errors, none of + /// them anywhere near an artifact - while every command, event and reactor is read exactly as written. Calling + /// that an error says something untrue about a document that is entirely correct, and makes the host throw it + /// away. + /// + /// So the severity follows how many artifacts were recovered from a declaration no compilation error sits inside. + /// None - because nothing was recovered at all, or because every declaration something came out of is one the + /// compiler could not make sense of - is an error, and nothing in the document is worth trusting. Any at all is a + /// warning saying how many came through, because an artifact read from source the compiler accepted describes + /// what that source states regardless of what failed elsewhere. + /// + /// + /// As an error it suppresses and , both for + /// the same reason: a model recovered from symbols the compiler never accepted describes an application that does + /// not exist, so an empty document and a rejected one are consequences of the broken build rather than defects of + /// their own. As a warning it suppresses neither - it can never coincide with the first, since a warning is only + /// reached when something was recovered, and suppressing the second would hand back a document the language + /// rejects with nothing wrong reported. + /// + /// public const string SourceDidNotCompile = "SP0024"; /// diff --git a/Source/DotNET/Screenplay/ScreenplayGenerator.cs b/Source/DotNET/Screenplay/ScreenplayGenerator.cs index 2140740c1..08b6b6a70 100644 --- a/Source/DotNET/Screenplay/ScreenplayGenerator.cs +++ b/Source/DotNET/Screenplay/ScreenplayGenerator.cs @@ -68,6 +68,13 @@ public ScreenplayGenerationResult Generate(Compilation compilation, ScreenplayOp /// way it suppresses : a model recovered from symbols /// the compiler never accepted describes an application that does not exist, so a poor document made from it is /// the consequence already reported rather than a second defect. + /// + /// The suppression follows the severity that code was reported at rather than the code alone. As an error it + /// says nothing recovered can be trusted, which is the whole reason a document built from it says nothing + /// either. As a warning it says the opposite - that what was recovered stands - and a document built from a + /// model that stands is exactly what this check is for. Suppressing it there would hand back a document the + /// language rejects with nothing wrong reported, which is the one outcome this exists to make impossible. + /// /// void ReportDocumentThatDoesNotCompile( string source, @@ -75,7 +82,7 @@ void ReportDocumentThatDoesNotCompile( ScreenplayDiagnostics diagnostics, string? location) { - if (analyzed.Any(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile)) + if (analyzed.Any(_ => _.Code == ScreenplayDiagnosticCodes.SourceDidNotCompile && _.Severity == ScreenplayDiagnosticSeverity.Error)) { return; }