Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ffigen] Add external-versions.*.max #1988

Merged
merged 6 commits into from
Feb 13, 2025
Merged

[ffigen] Add external-versions.*.max #1988

merged 6 commits into from
Feb 13, 2025

Conversation

liamappelbe
Copy link
Contributor

@liamappelbe liamappelbe commented Feb 12, 2025

Add a max field to the external-versions config, and extend the API versioning checks to check the max version vs the version that the API was introduced. Also, if an API is available in some versions, but not all of them, generate documentation describing the versions.

The main changes are in api_availability.dart. Instead of just returning a bool for whether the API is available, we now return an Availability enum, to also represent partial availability.

The version check itself has also become more complicated, since we're now checking the overlap of two ranges, instead of just checking one version vs a range. Broadly speaking, if apiMin <= configMin && apiMax >= configMax, the API is Availability.all, and if apiMin <= confMax && apiMax > confMin then the API is Availability.some, otherwise it's Availability.none. There are a bunch of other edge cases to deal with, but that's the main logic.

Details

  • If an interface or category is unavailable due to API versioning, but is depended on by other APIs, generate them as a stub. Don't omit them from the AST because it breaks things. Had to add an unavailable flag to their AST nodes, and use it in some of the transformers to get this behavior.
  • Had to refactor getCursorDocComment a bit to handle the versioning documentation.
  • Unrelated change: makeDartDoc is already null aware, so removed some if (dartDoc != null) checks.

Fixes #1422

Copy link

github-actions bot commented Feb 12, 2025

PR Health

Breaking changes ✔️
Package Change Current Version New Version Needed Version Looking good?
objective_c None 5.0.0 5.0.0 5.0.0 ✔️
Changelog Entry
Package Changed Files
package:objective_c pkgs/objective_c/lib/src/objective_c_bindings_generated.dart

Changes to files need to be accounted for in their respective changelogs.

This check can be disabled by tagging the PR with skip-changelog-check.

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.

Package Leaked API symbols
objective_c _Version

This check can be disabled by tagging the PR with skip-leaking-check.

License Headers ✔️
// Copyright (c) 2025, the Dart project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
Files
no missing headers

All source files should start with a license header.

Unrelated files missing license headers
Files
pkgs/jni/lib/src/third_party/generated_bindings.dart
pkgs/objective_c/lib/src/ns_input_stream.dart

@liamappelbe liamappelbe marked this pull request as ready for review February 13, 2025 03:58
@coveralls
Copy link

coveralls commented Feb 13, 2025

Coverage Status

coverage: 88.163%. first build
when pulling a758555 on extver
into 5e658f5 on main.

// to the other platforms.
if (minVer == null) {
Availability? availability;
for (final (plat, ver) in [(ios, iosVer), (macos, macosVer)]) {
Copy link
Member

Choose a reason for hiding this comment

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

Not much more characters to write platform and version and it follows the dart style.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@liamappelbe liamappelbe merged commit ddfb5b1 into main Feb 13, 2025
21 of 22 checks passed
@liamappelbe liamappelbe deleted the extver branch February 13, 2025 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ffigen] Add external-versions.*.max
3 participants