Skip to content

Migrate to the prototype visitor API - #1955

Draft
liamappelbe wants to merge 1 commit into
dart-lang:masterfrom
liamappelbe:ffigen_visitor_api_migration
Draft

Migrate to the prototype visitor API#1955
liamappelbe wants to merge 1 commit into
dart-lang:masterfrom
liamappelbe:ffigen_visitor_api_migration

Conversation

@liamappelbe

@liamappelbe liamappelbe commented Jul 24, 2026

Copy link
Copy Markdown

Context: dart-lang/native#3482

I'm not planning to merge this, just sharing what it would look like under the new API.

import 'package:logging/logging.dart';

class NativeCupertinoHttpVisitor extends Visitor {
static const includedInterfaces = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd probably inline this in the one method where it is used.

if (includedInterfaces.contains(node.originalName)) {
node.isExcluded = false;
for (final method in node.methods) {
if (node.originalName == 'NSURLResponse' &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe I'd try to make const maps with strings for the renames as well.

const methodRenames = {
  'NSURLResponse' : {
    'initWithURL:MIMEType:expectedContentLength:textEncodingName:':
       'initWithUrlAndMIMEType',
  },
  'NSHTTPURLResponse': // ...
}

and ditto for the param renames

That might be easier to read than the for loops.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Side note, we don't visit the methods and params and look up at the parents? That would avoid us having to have the for loops here. E.g. then we'd just reuse the visitParam and visitMethod that already traverse the whole ast.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package:cupertino_http Issues related to package:cupertino_http

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants