Skip to content

Fix member-level @available attributes hoisted to mock class declaration#352

Merged
sidepelican merged 8 commits into
uber:masterfrom
farkasseb:fix/member-available-attributes
Jun 13, 2026
Merged

Fix member-level @available attributes hoisted to mock class declaration#352
sidepelican merged 8 commits into
uber:masterfrom
farkasseb:fix/member-available-attributes

Conversation

@farkasseb

@farkasseb farkasseb commented May 24, 2026

Copy link
Copy Markdown
Contributor

Summary

Member-level behavioral @available attributes (noasync, deprecated) were concatenated and hoisted to the mock class declaration instead of staying on the individual generated members.

Example

/// @mockable
public protocol StorageInput: Sendable {
    func put(_ value: String, for key: String) async throws
    func string(for key: String) async throws -> String

    @available(*, noasync)
    @available(*, deprecated, message: "Use put(_:for:) async throws")
    @discardableResult func set(_ value: String, for key: String) -> Result<Void, Error>

    @available(*, noasync)
    @available(*, deprecated, message: "Use string(for:) async throws")
    func string(for key: String) -> Result<String, Error>
}

Before — attributes from all deprecated members are concatenated onto the class declaration, causing a build error:

@available(*, noasync)
    @available(*, deprecated, message: "Use put(_:for:) async throws")
    @discardableResult @available(*, noasync)
    @available(*, deprecated, message: "Use string(for:) async throws")
public final class StorageInputMock: StorageInput, @unchecked Sendable {
    public var setCallCount: Int { ... }
    public func set(_ value: String, for key: String) -> Result<Void, Error> { ... }
    public func string(for key: String) -> Result<String, Error> { ... }
}

After — only the protocol-conforming methods carry their own attributes:

public final class StorageInputMock: StorageInput, @unchecked Sendable {
    public var setCallCount: Int { ... }
    public var setHandler: (...)? { ... }
    @available(*, noasync)
    @available(*, deprecated, message: "Use put(_:for:) async throws")
    public func set(_ value: String, for key: String) -> Result<Void, Error> { ... }

    public var stringForCallCount: Int { ... }
    public var stringForHandler: (...)? { ... }
    @available(*, noasync)
    @available(*, deprecated, message: "Use string(for:) async throws")
    public func string(for key: String) -> Result<String, Error> { ... }
}

Attributes are placed only on the protocol-conforming declaration (funcs, vars, subscripts, and inits), not on mock infrastructure (callCount, handler, state). This keeps test setup and inspection clean — configuring a handler for a deprecated method won't trigger deprecation warnings, and noasync won't restrict mock configuration from async tests.

One deliberate consequence: a deprecated stored property still produces a deprecation warning inside the generated convenience init (self.data = data) — seeding a deprecated property is deprecated usage, and the warning correctly points at the protocol still carrying the deprecated member.

Note on platform availability

Existence-gating availability — @available(iOS 16, *), introduced:, obsoleted:, unavailable — continues to hoist to the class declaration, same as before this fix, since the mock's infrastructure references the member's types unconditionally. Extended-form platform-scoped deprecations (@available(iOS, deprecated: 12.0)) are warning-only, so they stay on the member like their *-form counterparts; when a single attribute combines both (@available(iOS, introduced: 10.0, deprecated: 12.0)), gating wins and it hoists.

Two smaller behavior notes: non-@available attributes no longer leak onto the class alongside a hoisted member attribute (only @available is ever hoisted), and behavioral attributes on associatedtypes are deliberately dropped, since the generated typealias is referenced throughout the mock's own infrastructure.

Hoisted class-level attributes are now deduplicated per individual attribute and rendered in member source order (order-preserving uniqued()); previously an unordered Set join made the order vary between executions.

Test plan

  • 4 focused test fixtures (compiled @Fixture mocks, so expected output is conformance-checked by the compiler): deprecated members on the original reported issue, protocol-level + member-level @available coexistence, multiple attributes on one method, and platform attributes hoisting to the class
  • All 155 tests pass

🤖 Generated with Claude Code

farkasseb and others added 3 commits June 10, 2026 17:38
Behavioral @available attributes (noasync, deprecated) on protocol
members were incorrectly placed on the generated mock class instead of
on the individual generated members. Platform @available correctly stays
on the class since stored properties cannot carry platform availability.

Split the attribute pipeline: behavioral @available flows to member
models (MethodModel, VariableModel) and is rendered on each generated
declaration; platform @available continues through the existing side
channel to the class.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Attributes like noasync and deprecated belong on the mock's public API
(the func/var that conforms to the protocol), not on mock infrastructure
(callCount, handler, state). Keeping infrastructure clean lets tests
configure and inspect deprecated methods without warnings or restrictions.

Also adds a duplicatedAttributes test covering the original reported
issue where multiple deprecated methods caused attributes to concatenate
onto the class declaration.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rwarding

Classify platform-scoped deprecations (@available(iOS, deprecated: 12.0))
as behavioral so they stay on the generated member instead of deprecating
the whole mock class; existence-gating forms (introduced:/obsoleted:/
unavailable, version shorthand) keep hoisting. Forward behavioral
attributes to generated required inits, consistent with all other member
kinds; document the deliberate drop on typealiases. Add fixtures for
mixed platform+behavioral, platform-scoped deprecation, gating
characterization, Combine/Rx vars, subscripts, and inits.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@farkasseb
farkasseb force-pushed the fix/member-available-attributes branch from 32f36f0 to e30a19c Compare June 11, 2026 00:35
farkasseb added a commit to farkasseb/mockolo that referenced this pull request Jun 11, 2026
Resolves the computed-var template conflict by combining uber#353's getter
call-count declaration with uber#352's attribute prefix. Also converts the
getter-history Combine fixture to the dummy Combine types that uber#352
introduced in Tests/Types.swift (extended with Published/DummyPublisher),
since the local dummies shadow the real Combine import — this also fixes
the fixture's Linux incompatibility.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@sidepelican sidepelican left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It looks good, but there are several improvements.

  • Create a Attribute struct and store the parsed information from AttributeList within [Attribute].
    • Model should have this as var attributes: [Attribute] instead of [String].
    • Add an applyAttributeTemplate extension to the [Attribute] type that functions similarly to the current asAttributePrefix implementation.
    • Remove isBehavioralAvailable, behavioralAvailableDescriptions, platformAvailableString, and platformAvailableDescriptions, and add computed properties to Attribute as needed
    • When generating objects like VariableModel, pass the attributes using syntax like attributes.filter(\.isAvailable).
  • Remove the redundant attributes parameter from applyFooTemplate and instead reference it directly from self.
  • Avoid adding unnecessary tests. Focus only on the minimum required tests to verify functionality.
  • Ensure that the generated output remains consistent across executions.

@sidepelican
sidepelican force-pushed the fix/member-available-attributes branch from 01e50aa to 0ab7643 Compare June 11, 2026 01:56
…stic

Addresses review feedback on uber#352:
- Parse AttributeList into plain Attribute values ([Attribute]) and plumb
  them end to end (parser -> models -> templates) instead of [String];
  Model now declares `var attributes: [Attribute]`
- Move @available classification into Attribute.Kind and drop the
  AttributeListSyntax helpers (isBehavioralAvailable,
  behavioralAvailableDescriptions, platformAvailableString,
  platformAvailableDescriptions); construction sites filter with
  attributes.filter(\.isBehavioralAvailable) / (\.isPlatformAvailable)
- Replace [String].asAttributePrefix with applyAttributeTemplate() on
  [Attribute]; apply*Template functions read self.attributes instead of
  taking a redundant parameter
- Replace the unordered Set dedupe of class-level hoisted attributes with
  order-preserving uniqued(), so output is consistent across executions
  and follows member source order; the memberPlatformAvailable fixture
  encoded one random Set ordering and is updated to source order
- Dedupe now operates per individual attribute rather than per-member
  joined strings (intentional)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@farkasseb

Copy link
Copy Markdown
Contributor Author

Thank you so much for the review @sidepelican ! I tried to implement what you were suggesting, please let me know what you think.

Regarding the excessive testing, the trim is fine by me, but to calibrate future PRs: how do you weigh scenario coverage against fixture maintenance? My instinct was that more covered scenarios is better.

@farkasseb
farkasseb requested a review from sidepelican June 12, 2026 02:12
@sidepelican

Copy link
Copy Markdown
Collaborator

Too many tests can increase maintenance costs.
When the purpose of a test is unclear, it can end up confusing developers.

@sidepelican sidepelican left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Since the parse logic felt a bit complex, I refactored it into a simpler version here. I believe this should be sufficient.
Thanks.

@sidepelican
sidepelican merged commit cadfa71 into uber:master Jun 13, 2026
8 checks passed
@farkasseb
farkasseb deleted the fix/member-available-attributes branch June 13, 2026 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants