pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.35k stars 1.44k forks source link

Generated compiler macro regression #2831

Closed stefancodinglands closed 8 months ago

stefancodinglands commented 8 months ago

Description

2827 This PR introduced a regression because Scope is not namespaced and it can collide with something else.

It impacts code like the following:

@Reducer
enum Path {
  case detail(DetailFeature)
  case meeting(MeetingFeature)
  case record(RecordFeature)
}
Screenshot 2024-02-19 at 12 22 35

The problem is the type of the body property:

public static var body: ReducerBuilder<Self.State, Self.Action>._Sequence<ReducerBuilder<Self.State, Self.Action>._Sequence<ReducerBuilder<Self.State, Self.Action>._Sequence<ReducerBuilder<Self.State, Self.Action>._Sequence<ReducerBuilder<Self.State,
        Self.Action>._Sequence<Scope<Self.State, Self.Action, AuthViewReducer>, Scope<Self.State, Self.Action, OnboardingReducer>>, Scope<Self.State, Self.Action, OnboardingQuestionnaireReducer>>, Scope<Self.State, Self.Action, RegisterEmailPassReducer>>, Scope<Self.State, Self.Action, ValidateEmailReducer>>, Scope<Self.State, Self.Action, LoginScreenReducer>> { }

However, changing the Scope to ComposableArchitecture.Scope fixes it. For example Scope<Self.State, Self.Action, AuthViewReducer> doesn't compile, but ComposableArchitecture.Scope<Self.State, Self.Action, AuthViewReducer>.

Checklist

Expected behavior

I expect the generated macro to be valid code as it was in 1.8.0

Actual behavior

The code doesn't compile anymore.

Steps to reproduce

Unfortunately I don't know how to reproduce it as I can't seem to find any other dependency in my project that provides a Scope definition.

The Composable Architecture version information

1.8.1

Destination operating system

iOS 17.2

Xcode version information

Version 15.2 (15C500b)

Swift Compiler version information

swift-driver version: 1.87.3 Apple Swift version 5.9.2 (swiftlang-5.9.2.2.56 clang-1500.1.0.2.5)
Target: arm64-apple-macosx14.0
Alex293 commented 8 months ago

I wonder if the type generation instead of some Reducer should be an option because it was written to avoid a bug and from a progressive disclosure point of view it push users who look at the generated code to not user some ReducerOf ?

stefancodinglands commented 8 months ago

I wonder if the type generation instead of some Reducer should be an option because it was written to avoid a bug and from a progressive disclosure point of view it push users who look at the generated code to not user some ReducerOf ?

Maybe that's an option too. However the fix is pretty simple, as I copied all the code generated by the macro, changed Scope to ComposableArchitecture.Scope and then everything worked.

This proposed change is in line with the way Scope is used inside the body generated by the macro (in both 1.8.0 and 1.8.1). See the image bellow, Scope is never used without the namespace.

Screenshot 2024-02-19 at 13 33 18

(This image is from 1.8.0 version, but the only thing that's different is the type of the body property, not the body itself)

Alex293 commented 8 months ago

I agree this should be fixed anyway

mbrandonw commented 8 months ago

Hi @stefancodinglands, thanks for the report. The fix is definitely to simply namespace the types in the macro expanded code. I accidentally left that off. I can get to it in a few hours, or you can PR a fix in the meantime.

mbrandonw commented 8 months ago

This has been fixed in 1.8.2.