[ffigen] propagate deprecated attributes into generated dart bindings#3301
[ffigen] propagate deprecated attributes into generated dart bindings#3301Hassnaa9 wants to merge 2 commits intodart-lang:mainfrom
Conversation
|
Hi @liamappelbe |
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with Breaking changes ✔️
This check can be disabled by tagging the PR with API leaks ✔️The following packages contain symbols visible in the public API, but not exported by the library. Export these symbols or remove them from your publicly visible API.
This check can be disabled by tagging the PR with |
liamappelbe
left a comment
liamappelbe
left a comment
There was a problem hiding this comment.
Looks pretty good. The CI failures are just outdated bindings. I'll regenerate them for you once we've resolved the other issues.
| final deprecatedPrefix = deprecatedAnnotation != null | ||
| ? '$deprecatedAnnotation\n ' | ||
| : ''; | ||
| s.write('\n $deprecatedPrefix${makeDartDoc(dartDoc)} '); |
There was a problem hiding this comment.
separate s.write calls would make more sense, like you have in the other cases.
| String? get deprecatedAnnotation { | ||
| if (!isDeprecated) return null; | ||
| final msg = _effectiveDeprecationMessage; | ||
| final escaped = msg.replaceAll(r'\', r'\\').replaceAll("'", r"\'"); |
There was a problem hiding this comment.
We already have a util for escaping strings, here. It operates on Pointer<Char>, so the easiest way to use it is probably while converting the native string to a dart string.
Another option would be to refactor that util so that it operates on Dart strings, and do the conversion as a separate step (that's probably the better design, but refactoring that function would be more work).
| if (deprecationMessage != null && deprecationMessage!.isNotEmpty) { | ||
| return deprecationMessage!; | ||
| } | ||
| final platformMsg = ios?.message ?? macos?.message; |
There was a problem hiding this comment.
If the messages are different, can you report both, with "iOS:" or "macOS:" at the start?
| expect( | ||
| trimmed, | ||
| contains(''' | ||
| /// DeprecatedInterface |
| final trimmed = bindings.split('\n').map((l) => l.trim()).join('\n'); | ||
| expect( | ||
| trimmed, | ||
| isNot(contains("@Deprecated('Deprecated')\nint normalMethod")), |
There was a problem hiding this comment.
multi-line strings are easier to read here. Like the other tests.
2 participants
Fixes #3020
Propagates @deprecated('...') annotations from C/ObjC headers into generated Dart bindings.