[Docs] Add a section to Diagnostics.md on -verify mode #27448
Conversation
|
cc @jrose-apple |
|
Looks good to me. I think you're right about Diagnostics.md too, but maybe you could throw a cross-reference into Testing.md? |
owenv
force-pushed
the
verifier_docs
branch
2 times, most recently
from
4ec3811 to
c847ad3
Compare
|
@jrose-apple sounds good to me, done! |
|
@swift-ci Please smoke test and merge |
|
|
||
| - (Required) The expected error message. The message should be enclosed in double curly braces and should not include the `error:`/`warning:`/`note:`/`remark:` prefix. For example, `// expected-error {{invalid redeclaration of 'y'}}` would match an error with that message on the same line. The expected message will match so long as it's a substring of the message which is emitted. | ||
|
|
||
| - (Optional) Expected fix-its. These are each enclosed in double curly braces and appear after the expected message. An expected fix-it consists of a column range followed by the text it's expected to be replaced with. For example, `let r : Int i = j // expected-error{{consecutive statements}} {{12-12=;}}` will match a fix-it attached to the consecutive statements error which inserts a semicolon at column 12, just after the 't' in 'Int'. The special {{none}} specifier is also supported, which will cause the diagnostic match to fail if unexpected fix-its are produced. |
There was a problem hiding this comment.
I’m still confused about this - the error is consecutive statements on a line must be separated by ';', but it seems you can just enter the first few words to match it. Does it only apply to errors/warnings/notes that are unique (i.e unique prefix perhaps?) or is there something else you need to do for this to work? I think we should document it as well.
There was a problem hiding this comment.
I referenced that briefly above: "The expected message will match so long as it's a substring of the message which is emitted.". In practice I think it's only ever used to match a prefix, so I may look into tightening that restriction if it doesn't require updating too many tests.
There was a problem hiding this comment.
I clarified this a little more in the meantime to hopefully avoid confusion though.
There was a problem hiding this comment.
Thanks! I think we should match unique prefixes otherwise if you had two or more diagnostics with the same prefix then it won’t be clear which one is being referred to in the tests.
(although I haven’t seen many tests utilising this and the ones that are don’t seem to conflict with other diagnostics so I’m not sure if this is really a problem, at least for now).
There was a problem hiding this comment.
It's definitely just matching a substring, not a prefix. Sometimes the interesting words in a diagnostic are in the middle, and/or you don't want to copy out a whole type or something when it's not relevant to the test.
|
@swift-ci Please smoke test |
|
@swift-ci Please smoke test macOS |
As a follow up to adding column specification to the diagnostics verifier I've been meaning to document the
// expected-syntax now that it's gotten a bit more complicated. Even though a lot of this can be determined by inspection, I think it's useful to have a short reference for new contributors and people encountering features like{{none}}and*for the first time.I suppose this could go in
Testing.md, butDiagnostics.mdseemed like a better fit since it includes more of the relevant terminology.