Add #examples and #corrections macros to DRY up rule description#6794
Add #examples and #corrections macros to DRY up rule description#6794ZevEisenberg wants to merge 9 commits into
#examples and #corrections macros to DRY up rule description#6794Conversation
Generated by 🚫 Danger |
ZevEisenberg
changed the title
#examples and #examplesDictionary macros to DRY up rule description
SimplyDanny
left a comment
SimplyDanny
left a comment
There was a problem hiding this comment.
Thank you, @ZevEisenberg, for keeping an eye on test example setup over the years! 👍
A few thoughts, remarks and ideas from my side:
- Ideally, protocol conformances support default parameters at some point. Then we can make
ExampleExpressibleByStringLiteraland so avoid any manual conversion. - I'd prefer to only have a single macro
#examplesthat accepts either an array or a dictionary. Usage should be clear from the context of usage alone. - It might be worth supporting some of
Examples parameters as macro arguments as well. There are a few cases where a parameter must be applied to all examples or at least a subset of them. - To further reduce boilerplate, we could try to get rid of the brackets wrapping the examples by using variadic parameters. At least for arrays, that should work.
- The dictionary case is a little more tricky. Instead of a dictionary, we could have a list of
CorrectionExampleobjects that wrap twoExamples. They could be created with a custom operator=>, e.g.corrections: #examples( "let foo = 1" => "let bar = 1", ... )
| let example: ExprSyntax = if let fileID = context.location( | ||
| of: expression, at: .afterLeadingTrivia, filePathMode: .fileID | ||
| ), let filePath = context.location( | ||
| of: expression, at: .afterLeadingTrivia, filePathMode: .filePath | ||
| ) { |
There was a problem hiding this comment.
That's a bit hard to read. I'd separate declaration from optional binding in the if expression. And actually, it should be an error if there's no source location.
Co-authored-by: Danny Mösch <danny.moesch@icloud.com>
@SimplyDanny can you say more? Does this affect any code in the project today, or is it hypothetical? |
Co-authored-by: Danny Mösch <danny.moesch@icloud.com>
Oh, actually, I see what you mean. I had missed a bunch of places where we call |
|
Another one I'm seeing in DuplicateImportsRuleExamples.swift: Example("""
↓import A.B.C
↓import A.B
import A
""", excludeFromDocumentation: true): Example("""
import A
"""),The approach I'm currently trying: making I'll also revisit the macro names as you suggest. What do you think about changing the dictionary one to |
Also fine. However, just using |
|
One other option I'm thinking about, and I'd love to get your take on: it's going to be annoying to have two different styles for examples: #examples([
"Some example",
Example("exclude this one", excludeFromDocumentation: true),
])I'm wondering what you might think about using a fluent API like this: #examples([
"Some example",
"exclude this one".excludeFromDocumentation(),
])There could also be a function for overriding |
This is ready for another review! |
ZevEisenberg
force-pushed
the
examples-macro
branch
from
c6f47e2 to
78a2147
Compare
…rt as many remaining Example call sites into bare strings.
… Xcode syntax highlighting and makes the Expand Macro command show up and work more reliably.
ZevEisenberg
force-pushed
the
examples-macro
branch
from
78a2147 to
71690c3
Compare
|
Fixed a build failure |
ZevEisenberg
changed the title
#examples and #examplesDictionary macros to DRY up rule description#examples and #corrections macros to DRY up rule description
3 participants
When I wrote the
Exampletype in #3040, it improved the experience of writing and debugging a rule (because test failures would point at precisely the example case that failed the test). It did this by bundling up the file and line number for each example, which can be passed along to the test assertion functions. But it came at the expense of a lot of repetitiveExample("...")call sites.Now that macros exist, I propose using them to keep the benefits of
Examplewhile cleaning up the call sites. The new macros transform[String]and[String: String]to[Example]and[Example: Example], respectively.If it's helpful for review, here's a Claude-generated script that confirms that the changes to the rule files are just adding the macro and removing the
Examplewrapper:review_mechanical_changes.py
Alternatives/Questions
replacethe entire init of RuleDescription. But this seemed like an overreach, and would prevent falling back to passing variables or non-literals, which limits flexibility and experimentation.#examples, but that seemed too cute and not clear enough at the point of use.