Skip to content

feat(hermtic): fix some edge cases and cleanup bootstrapping after ne…#14

Merged
crazycrank merged 1 commit into
masterfrom
feat/sdk-rework
Apr 12, 2026
Merged

feat(hermtic): fix some edge cases and cleanup bootstrapping after ne…#14
crazycrank merged 1 commit into
masterfrom
feat/sdk-rework

Conversation

@crazycrank

Copy link
Copy Markdown
Contributor

…w sdk release

Copilot AI review requested due to automatic review settings April 12, 2026 19:39
@crazycrank crazycrank merged commit 883bdde into master Apr 12, 2026
1 check passed
@crazycrank crazycrank deleted the feat/sdk-rework branch April 12, 2026 19:45

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR expands Scribe Shapes with additional “phase 8/8.5” primitives and corresponding code fixes, and introduces a custom Fix All implementation to correctly apply multiple Shape fixes that target the same type declaration.

Changes:

  • Add new ShapeBuilder constraints (new MustBe*/MustNot* primitives) and extend the fix-kind catalog to support them.
  • Add/extend code-fix infrastructure in Scribe.Ink (new fix implementations, shared fix resolver, and a custom FixAllProvider to chain edits per type).
  • Add comprehensive tests covering new diagnostics, fix kinds, and Fix All chaining behavior.

Reviewed changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
Scribe/Shapes/ShapeBuilder.cs Adds additional Shape primitives and predicate helpers.
Scribe/Shapes/FixSpec.cs Extends FixKind to represent new fix operations.
Scribe.Ink/Shapes/ShapeFixAllProvider.cs Custom Fix All provider to chain multiple fixes on the same type.
Scribe.Ink/Shapes/ShapeCodeFixProvider.cs Routes diagnostics to fix implementations and selects Fix All provider.
Scribe.Ink/Shapes/FixResolver.cs Central mapping from FixKindIShapeFix.
Scribe.Ink/Shapes/Fixes/* Implements the new concrete fixes (visibility, modifier removal, base list edits, ctor insertion, etc.).
Scribe.Tests/Shapes/ShapePhase8PrimitivesTests.cs Tests for earlier phase primitives + fixes.
Scribe.Tests/Shapes/ShapePhase8_5Tests.cs Tests for phase 8.5 primitives + fixes.
Scribe.Tests/Shapes/ShapeFixAllProviderTests.cs Validates Fix All chaining across multiple diagnostics on the same declaration.
Scribe.Sdk/Sdk/Phases/*.targets Updates SDK behavior (Meta/Analyzer/etc. defaults).
global.json / Directory.Build.props / *.csproj Build/SDK bootstrapping and defaults adjustments.
docs/project-setup.md Updates setup guidance to reflect SDK/build changes.
Comments suppressed due to low confidence (1)

Scribe.Ink/Shapes/ShapeCodeFixProvider.cs:77

  • The fix provider currently uses BatchFixer and a local ResolveFix switch that only handles a subset of FixKind values. With the newly added fixes/tests (abstract/static/base class/removals/visibility/ctor), diagnostics will either not offer code fixes or Fix All won’t chain them safely. Wire GetFixAllProvider() to the custom ShapeFixAllProvider and centralize fix resolution through FixResolver so all FixKind values are supported consistently.
    public override FixAllProvider? GetFixAllProvider() => ShapeFixAllProvider.Instance;

    public override async Task RegisterCodeFixesAsync(CodeFixContext context)
    {
        var root = await context.Document.GetSyntaxRootAsync(context.CancellationToken)
            .ConfigureAwait(false);
        if (root is null)
        {
            return;
        }

        foreach (var diagnostic in context.Diagnostics)
        {
            if (!diagnostic.Properties.TryGetValue("fixKind", out var fixKindStr)
                || string.IsNullOrEmpty(fixKindStr)
                || !Enum.TryParse<FixKind>(fixKindStr, out var fixKind)
                || fixKind == FixKind.None)
            {
                continue;
            }

            var fix = FixResolver.Resolve(fixKind);
            if (fix is null)
            {
                continue;
            }

            var node = root.FindNode(diagnostic.Location.SourceSpan);
            var typeDecl = node.AncestorsAndSelf().OfType<TypeDeclarationSyntax>().FirstOrDefault();
            if (typeDecl is null)
            {
                continue;
            }

            context.RegisterCodeFix(
                CodeAction.Create(
                    title: fix.Title(diagnostic),
                    createChangedDocument: ct => fix.FixAsync(context.Document, typeDecl, diagnostic, ct),
                    equivalenceKey: fixKind.ToString()),
                diagnostic);
        }
    }

}

Comment on lines +49 to +71
private static async Task<Solution> FixAllAsync(FixAllContext context, CancellationToken ct)
{
var solution = context.Solution;

var documents = CollectDocuments(context, ct);

foreach (var document in documents)
{
ct.ThrowIfCancellationRequested();
var diagnostics = await context.GetDocumentDiagnosticsAsync(document).ConfigureAwait(false);
if (diagnostics.IsDefaultOrEmpty)
{
continue;
}

var updated = await FixDocumentAsync(document, diagnostics, ct).ConfigureAwait(false);
if (updated is null)
{
continue;
}

solution = updated.Project.Solution;
}

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix-all currently applies edits to the original Document instances for project/solution scopes. Because FixDocumentAsync returns a Document from the original solution, updating solution = updated.Project.Solution doesn’t affect the next iteration, so earlier document changes can be lost (only the last processed document remains). Iterate over DocumentIds (or re-fetch the current document via solution.GetDocument(document.Id) each loop) and pass that into FixDocumentAsync so edits accumulate on the same evolving Solution.

Copilot uses AI. Check for mistakes.
Comment on lines +112 to +131
// Group diagnostics by the type declaration they target, then annotate
// each type so we can re-locate it after every edit.
var perType = new Dictionary<TypeDeclarationSyntax, List<Diagnostic>>();
foreach (var diagnostic in diagnostics)
{
ct.ThrowIfCancellationRequested();
var node = root.FindNode(diagnostic.Location.SourceSpan);
var typeDecl = node.AncestorsAndSelf().OfType<TypeDeclarationSyntax>().FirstOrDefault();
if (typeDecl is null)
{
continue;
}

if (!perType.TryGetValue(typeDecl, out var list))
{
list = new List<Diagnostic>();
perType[typeDecl] = list;
}
list.Add(diagnostic);
}

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Diagnostics within each annotated type are applied in whatever order Roslyn returns them, which can make Fix All results non-deterministic and can break when one fix changes the shape of the node another fix expects. Sort the per-type diagnostics (e.g., by Location.SourceSpan.Start, and optionally by fixKind) before applying them so Fix All behaves deterministically.

Copilot uses AI. Check for mistakes.
Comment on lines +24 to +31
FixKind.RemovePartialModifier => new RemovePartialModifierFix(),
FixKind.RemoveSealedModifier => new RemoveSealedModifierFix(),
FixKind.RemoveStaticModifier => new RemoveStaticModifierFix(),
FixKind.RemoveAttribute => new RemoveAttributeFix(),
FixKind.SetVisibility => new SetVisibilityFix(),
FixKind.AddParameterlessConstructor => new AddParameterlessConstructorFix(),
_ => null,
};

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FixResolver references FixKind members (e.g., RemovePartialModifier/SetVisibility/AddParameterlessConstructor) that are not present in Scribe.Shapes.FixKind in this branch, which will not compile and also prevents diagnostics from encoding these fix kinds consistently. Update the FixKind enum (and any analyzer emission) so every case handled here exists and is stable.

Suggested change
FixKind.RemovePartialModifier => new RemovePartialModifierFix(),
FixKind.RemoveSealedModifier => new RemoveSealedModifierFix(),
FixKind.RemoveStaticModifier => new RemoveStaticModifierFix(),
FixKind.RemoveAttribute => new RemoveAttributeFix(),
FixKind.SetVisibility => new SetVisibilityFix(),
FixKind.AddParameterlessConstructor => new AddParameterlessConstructorFix(),
_ => null,
};
FixKind.RemoveSealedModifier => new RemoveSealedModifierFix(),
FixKind.RemoveStaticModifier => new RemoveStaticModifierFix(),
FixKind.RemoveAttribute => new RemoveAttributeFix(),
_ => ResolveByName(kind),
};
private static IShapeFix? ResolveByName(FixKind kind)
{
var kindName = kind.ToString();
if (kindName == "RemovePartialModifier")
{
return new RemovePartialModifierFix();
}
if (kindName == "SetVisibility")
{
return new SetVisibilityFix();
}
if (kindName == "AddParameterlessConstructor")
{
return new AddParameterlessConstructorFix();
}
return null;
}

Copilot uses AI. Check for mistakes.
@@ -0,0 +1,48 @@
using System.Linq;

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using System.Linq; is unused in this file. Remove it to satisfy code-style/IDE analyzers and keep the fix implementation minimal.

Suggested change
using System.Linq;

Copilot uses AI. Check for mistakes.
Comment on lines +1 to +5
using System;
using System.Collections.Generic;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

using System.Linq; is unused in this file. Remove it to satisfy code-style/IDE analyzers.

Copilot uses AI. Check for mistakes.
Comment on lines +34 to +52
[Fact]
public void MustNotBePartial_emits_SCRIBE013_with_RemovePartialModifier_fix()
{
var shape = Shape.Class()
.MustNotBePartial()
.Project<Collected>((in ShapeProjectionContext ctx) => new Collected(ctx.Fqn));

var diagnostics = RunAnalyzer(shape, "public partial class Widget { }");
diagnostics.Length.ShouldBe(1);
diagnostics[0].Id.ShouldBe("SCRIBE013");
diagnostics[0].Properties["fixKind"].ShouldBe("RemovePartialModifier");
}

[Fact]
public async Task MustNotBePartial_fix_removes_partial_keyword()
{
var shape = Shape.Class()
.MustNotBePartial()
.Project<Collected>((in ShapeProjectionContext ctx) => new Collected(ctx.Fqn));

Copilot AI Apr 12, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test suite depends on new ShapeBuilder primitives like MustNotBePartial, MustNotBeSealed, MustBePublic, etc. Those APIs (and their SCRIBE013+ diagnostics) don’t exist in Scribe/Shapes/ShapeBuilder.cs in the current branch, so the test project won’t compile. Ensure the corresponding ShapeBuilder methods + diagnostic IDs are included in the PR (or adjust the tests to match the actual API surface).

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants