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

Resolved Types lost when using xcodeproj integration instead of sources #887

Closed
samadhiBot opened this issue Dec 22, 2020 · 6 comments
Closed

Comments

@samadhiBot
Copy link
Contributor

What version of Swift are you using (swift --version)?
Swift version: 5.3.1

What did you do?
Updated Sourcery from 0.18.0 to 1.0.2

What did you expect to see?
Actual types resolved as they were in 0.18.0

What did you see instead?
Certain actual types fail to resolve in 1.0.0 and later.

The issue appears to have been introduced in PR 834: Fixed namespace resolution for types. It does not seem to be a problem when specifying sources in the config.

But when specifying project -> target -> module in the configuration, the keying change in Composer.uniqueTypesAndFunctions from unique[type.name] to unique[type.globalName] prevents later Type lookups in Composer.actualTypeName from resolving. In effect, the key contains the module name, but later lookups do not.


The following demonstrates the issue.

Foo.swift:

struct Foo {
    struct Bar {
        let bat: Int
    }

    let bar: Bar
}

Stats.stencil:

// Found {{ types.all.count }} Types
{% for type in types.all %}

// - {{ type.name }} ({{ type.globalName }})
//     Variables for Type {{ type.name }}:
{%     for variable in type.variables %}
//     - {{ variable.name }}:
//       - typeName: {{ variable.typeName }}
//       - type: {{ variable.type.name }}
{%     endfor %}
{% endfor %}

sample-sources.yml:

sources:
    - ../../Foo
templates:
    - ../Templates/Stats.stencil
output: ../../Generated/
verbose: true

sample-targets.yml:

project:
  file: ../../Foo/Foo.xcodeproj
  target:
    name: Foo
templates:
    - ../Templates/Stats.stencil
output: ../../Generated/
verbose: true

Output based on sample-sources.yml:

// ./Sourcery/Sourcery-0.18.0/sourcery --config Sourcery/Configs/sample-sources.yml

// Generated using Sourcery 0.18.0 — https://github.com/krzysztofzablocki/Sourcery
// DO NOT EDIT

// Found 2 Types

// - Foo (Foo)
//     Variables for Type Foo:
//     - bar:
//       - typeName: Bar
//       - type: Foo.Bar

// - Foo.Bar (Foo.Bar)
//     Variables for Type Foo.Bar:
//     - bat:
//       - typeName: Int
//       - type: 

// ----------------------------------------------------------------------------------

// ./Sourcery/Sourcery-1.0.2/sourcery --config Sourcery/Configs/sample-sources.yml

// Generated using Sourcery 1.0.2 — https://github.com/krzysztofzablocki/Sourcery
// DO NOT EDIT

// Found 2 Types

// - Foo (Foo)
//     Variables for Type Foo:
//     - bar:
//       - typeName: Bar
//       - type: Foo.Bar

// - Foo.Bar (Foo.Bar)
//     Variables for Type Foo.Bar:
//     - bat:
//       - typeName: Int
//       - type: 

Output based on sample-targets.yml:

// ----------------------------------------------------------------------------------

// ./Sourcery/Sourcery-0.18.0/sourcery --config Sourcery/Configs/sample-targets.yml

// Generated using Sourcery 0.18.0 — https://github.com/krzysztofzablocki/Sourcery
// DO NOT EDIT

// Found 2 Types

// - Foo (Foo.Foo)
//     Variables for Type Foo:
//     - bar:
//       - typeName: Bar
//       - type: Foo.Bar

// - Foo.Bar (Foo.Foo.Bar)
//     Variables for Type Foo.Bar:
//     - bat:
//       - typeName: Int
//       - type: 

// ----------------------------------------------------------------------------------

// Generated using Sourcery 1.0.2 — https://github.com/krzysztofzablocki/Sourcery
// DO NOT EDIT

// Found 2 Types

// - Foo (Foo.Foo)
//     Variables for Type Foo:
//     - bar:
//       - typeName: Bar
//       - type: 

// - Foo.Bar (Foo.Foo.Bar)
//     Variables for Type Foo.Bar:
//     - bat:
//       - typeName: Int
//       - type: 

In the final sample, using Sourcery 1.0.2 and specifying a project -> target -> module configuration, the type for the bar variable fails to resolve. It’s keyed in Sourcery’s unique types as Foo.Foo.Bar (where the first Foo is the module), but subsequent attempts to resolve elements to the type are looking it up under Foo.Bar.


I’ve made some progress trying to resolve the issue, but once I got to generics types I started going in circles. Specifically, attempts to fix resolution of custom generic types along the lines of Set<Foo> would break resolution of other generics like Set<Double>, whenever we'd extended a built-in type (Double in this example).

The following changes cleaned up a big chunk of the problems in the generated code I’m working with, but this is an incomplete (and currently untested) solution.

diff --git a/SourceryFramework/Sources/Parsing/Composer.swift b/SourceryFramework/Sources/Parsing/Composer.swift
index bd8c03aa5d5ba1cfb3ceb72d50bd6f8c2827ecdd..86850eac60aa4705970fa0bcdc707978003c5f21 100644
--- a/SourceryFramework/Sources/Parsing/Composer.swift
+++ b/SourceryFramework/Sources/Parsing/Composer.swift
@@ -325,18 +325,25 @@ public struct Composer {
 
                 actualTypeName = containedType?.name ?? possibleTypeName
             } else {
-                if let name = unique?["\(containingType.name).\(unwrappedTypeName)"]?.name {
+                if let name = unique?["\(containingType.globalName).\(unwrappedTypeName)"]?.globalName {
                     //check contained types first
                     actualTypeName = name
                 } else {
                     //otherwise go up contained types chain to find a type
                     let parentTypes = containingType.parentTypes
                     while let parent = parentTypes.next() {
-                        if let name = unique?["\(parent.name).\(unwrappedTypeName)"]?.name {
+                        if let name = unique?["\(parent.globalName).\(unwrappedTypeName)"]?.globalName {
                             actualTypeName = name
                             break
                         }
                     }
+
+                    if actualTypeName == nil {
+                        //check with module name
+                        if let module = containingType.module, let name = unique?["\(module).\(unwrappedTypeName)"]?.globalName {
+                            actualTypeName = name
+                        }
+                    }
                     actualTypeName = actualTypeName ?? unwrappedTypeName
                 }
             }
@balavor
Copy link

balavor commented Jan 10, 2021

@krzysztofzablocki @ilyapuchka Hey guys, would you mind paying attention to that issue, please?

I've also faced that issue when I've tried to migrate from 0.18.0 to 1.0.0. I have a Prism template for my models with nested structure, like:

struct Model: Equatable {
  let kind: Kind; enum Kind: Equatable, AutoPrism {
    case one, two
  }
}

And I expect to have generated prisms as:

extension Model.Kind {
  var isOne: Bool { ... }
  var isTwo: Bool { ... }
}

Instead, I'm getting a compile error, because the generated code now creates an extension with only Kind specified as a name:

extension Kind { // Error: No such type 'Kind'
  var isOne: Bool { ... }
  var isTwo: Bool { ... }
}

Could you please tell me where I can find an appropriate spec to create a test case that will reproduce that issue?

@krzysztofzablocki
Copy link
Owner

krzysztofzablocki commented Jan 12, 2021

@RomanTysiachnik somewhere along ComposerSpec.swift:L1121

@krzysztofzablocki
Copy link
Owner

krzysztofzablocki commented Jan 12, 2021

@samadhiBot I've added a spec that I think reproduces your setup and your changes fix it, do you mind providing an example of what you meant about those changes breaking other functionality?

https://github.com/krzysztofzablocki/Sourcery/compare/feature/fix-module-type-resolution#diff-5ee1031640bdafff8116401894a2ce97a0a6dcc940b642d8ec5de522aa489da2R1177

@samadhiBot
Copy link
Contributor Author

@krzysztofzablocki I'm still working on an example to illustrate the breaking changes with Generics. Will do my best to get it to you this week.

@krzysztofzablocki krzysztofzablocki changed the title Resolved Types lost when specifying Modules Resolved Types lost when using xcodeproj integration instead of sources Jan 22, 2021
@samadhiBot
Copy link
Contributor Author

@krzysztofzablocki I've pushed up at sample project to https://github.com/samadhiBot/sourcery-sample. It adds a custom generic type the example I used above, and I believe this should cover the remaining problems I alluded to earlier.

The problem I was running into while trying to solve type lookup for generic types was that I could fix the Baz<Bar> type lookup, but that would break the Baz<Double> type lookup. When I fixed the Baz<Double> type lookup, it broke the Baz<Bar> type lookup.

If I remember correctly, having extensions on built-in types like Double was adding to the complexity, because it brought Double into Sourcery's purview. So my example contains a property of type Baz<Double> and another of type Baz<Int>, where Double has an extension and Int does not.

Thank you again for looking into this, and let me know if I can try to clarify anything further.

@krzysztofzablocki
Copy link
Owner

krzysztofzablocki commented Jan 27, 2021

@samadhiBot thanks, I added test case for that scenario and I also run my current version against the sample and I get the expected results

krzysztofzablocki added a commit that referenced this issue Jan 28, 2021
* chore(deps): bump redcarpet from 3.5.0 to 3.5.1

Bumps [redcarpet](https://github.com/vmg/redcarpet) from 3.5.0 to 3.5.1.
- [Release notes](https://github.com/vmg/redcarpet/releases)
- [Changelog](https://github.com/vmg/redcarpet/blob/master/CHANGELOG.md)
- [Commits](vmg/redcarpet@v3.5.0...v3.5.1)

Signed-off-by: dependabot[bot] <support@github.com>

* tests: add failing specs for modules issue #887

* refactor: apply samadhiBot changes

* chore: update changelog

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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

No branches or pull requests

3 participants