For the purposes of a proposal, you can omit the body of the method

These say report() but the actual proposed method is named record(), can you update those to be consistent throughout the document?

This property should probably stay SPI for now as it's a bit hacky and probably not the final design for this data.

This property should probably stay SPI for now as it's a bit hacky and probably not the final design for this data.

I don't think we really discuss events or event handlers in our API documentation because they're not otherwise part of our API surface. How about simply:

/// Record this issue.

We should make sure the API documentation explains what might be modified (the SPI documentation is, and this is a technical term here, "lazy".)

We should leave .system as SPI for now since we don't generally want external components to generate system issues. These represent problems with Swift Testing itself. (We could maybe add an additional case e.g. case integratedToolFailed(description: String), what do you think @stmontgomery ?)

Just so I understand, you're saying this line is correct? Or is there a more-correct @_spi annotation than ForToolsIntegrationOnly? The SPI doc only lists Experimental and ForToolsIntegrationOnly.

Yes, adding the SPI marker here is probably the right thing to do, but I'd like @stmontgomery to chime in too.

I think it's OK to leave .system without an @_spi attribute. Although it's true that users shouldn't generally record system issues themselves, I do think it can be useful for them to match against issues of that kind using other APIs like withKnownIssue. So personally, I don't think we need to add it as part of this proposal/PR.

This is fine but will produce a source context for the issue that points into Swift Testing rather than the call site, which is an oversight of the existing code to be fair. As well, we're contemplating lowering Backtrace and SourceContext to SPI since the Swift standard library has its own backtrace type now that we'll eventually want to adopt and we don't want a naming conflict.

How about we expose a different initializer with the signature:

init(
  _ kind: Kind,
  comments: [Comment] = [],
  sourceLocation: SourceLocation = #_sourceLocation
)

(Whether or not kind deserves a label is something we can bikeshed.)

Proposals don't need to include the bodies of methods/initializers/etc. You can leave the squiggly brackets out here.

It is intentional that Expectation does not have a public initializer: it is always supposed to be the product of #expect or #require. If you record an issue that does not come from one of those macros, it should not be producing an instance of this type.

See my alternative suggestion

Jonathan. :)

I'd like to explore ideas for ways to structure this that avoid needing to publicly expose an init on Expectation. I know I brought that idea up in an earlier comment on this PR, but after reviewing this type again, and taking @grynspan 's comments about the two properties above into consideration, I'm wary to open this type up for general API use right now until it's a bit better designed.

As one alternative, I wonder if we could expose a static func on Issue.Kind which takes a generic message, as well as one or more values of the evaluated expression, and internally construct an Expectation with the relevant Expression and Expression.Value values representing all of the passed-in values? Something like

extension Issue.Kind {
  public static func expectationFailed<each T>(
    _ description: String,
    values: repeat each T,
    isRequired: Bool,
    sourceLocation: SourceLocation
  ) -> Self
}

This would require some internal change to __Expression but that seems doable, and it would allow capturing rich details of the left- and right-hand side values of binary expressions.