nalexn / ViewInspector

Runtime introspection and unit testing of SwiftUI views
MIT License
2.09k stars 145 forks source link

Inspect fails to find views due to `guardType(value:namespacedPrefixes:inspectionCall:)` incorrectly failing #268

Open cameroncooke opened 8 months ago

cameroncooke commented 8 months ago

String extension hasPrefix(regex:wholeMatch:) doesn't handle reserved RegEx symbols and causes false negatives when checking type prefixes.

This is especially true when the type's description includes context information like module name, as an example:

(extension in MyModule):MyModule.MyComponent

Currently, in my project, I get the error:

found (extension in MyModule):MyModule.MyComponent instead of (extension in MyModule):MyModule.MyComponent

This in fact of course is false, the two type strings match, the issue is the injecting of the string and treating it as regex.

The Regex expression (extension in MyModule):MyModule.MyComponent does not have any matches for the literal String (extension in MyModule):MyModule.MyComponent.

The reason for this is:

Reproduction

In my case, the issue was caused because my view type was created within an extension in order to namespace it, i.e:

extension SomeType {
    struct MyComponent: View {
        ....
    }
}

Regex

Also while investigating this issue the code below seems a little questionable:

static var swiftUINamespaceRegex: String { "SwiftUI(|[a-zA-Z0-9]{0,2}\\))\\." }

func removingSwiftUINamespace() -> String {
    return removing(prefix: .swiftUINamespaceRegex)
        .removing(prefix: "SwiftUI.")
}

func removing(prefix: String) -> String {
    guard hasPrefix(prefix) else { return self }
    return String(suffix(count - prefix.count))
}

Again injecting regex into String.hasPrefix(_:) in the removing(prefix:) method via the removingSwiftUINamespace() method and counting the literal "prefix" characters to return a substring.

I did consider opening a pull request and working on a fix but I don't have enough knowledge around why some of this works the way it does to feel confident about changing it.

I did however create three unit tests to exercise the issues:

BaseTypesTests.swift:

    func testContainsPrefixRegex() {
        // Arrange
        let prefixes = ["MyModule.MyComponent"]

        // Assert
        XCTAssertTrue(prefixes.containsPrefixRegex(matching: "MyModule.MyComponent"))
    }

    func testContainsPrefixRegexWithContext() {
        // Arrange
        let prefixes = ["(extension in MyModule):MyModule.MyComponent"]

        // Assert
        XCTAssertTrue(prefixes.containsPrefixRegex(matching: "MyModule.MyComponent"))
    }

    func testRemovingSwiftUINamespace() {
        // Arrange
        let sut = "_CoreLocationUI_SwiftUI._MKMapGestureModifier"

        // Assert
        XCTAssertEqual(sut.removingSwiftUINamespace(), "_MKMapGestureModifier")
    }

The last two tests fail, also the last test is an assumption of what the code is there for.

nalexn commented 8 months ago

Hey @cameroncooke I'm trying to replicate the issue you're facing, tried various ways of defining views in extensions, including in a local .framework bundle - the view is unwrapped with no issues. Could you share more details on how that view is defined? Or attach a sample project setup that recreates the issue. I could have just corrected the code of containsPrefixRegex to make the tests pass, but I'm not 100% sure that's the right place to address the issue.

nalexn commented 8 months ago

Usually reflection shows the namespaces of custom types as (unknown context), thus a parser, I wonder if you have something peculiar in your setup that made reflection work differently and actually render the namespace like you say (extension in MyModule). Maybe you're on a beta Xcode version?

nalexn commented 8 months ago

You can also prepare a PR. I would advise adding tests that verify Inspector.typeName(type: MyComponent.self, namespaced: true) is removing the (extension in MyModule) part and return MyModule.MyComponent. Under the hood that method actually calls sanitizingNamespace, which is the function where the fix should go.

nalexn commented 8 months ago

The related code was updated in #273 , so could you uncomment this line and verify it's working as expected for you?