Periphery gives inconsistent results after 2.18.0 update

[mildm8nnered]

I have a largish codebase (hundreds of thousand of lines of code). When I run periphery it detects 1,443 warnings. When I run it again, it detects 1,446 warnings.

The three extra warnings are always the same - they're unused properties, all in one file.

Frequency this happens is that if I run Periphery once, twice or three times, I might get identical answers. But if I run four times, at least one will be different.

I'm pretty sure that this came in with 2.18.0 - I can't make it happen with earlier versions.

I haven't tried to track down what's happening in the code, but I may be able to git bisect to see where this was introduced. Posting this here for now, in case this rings any bells.

[mildm8nnered]

So according to a git bisect run

$ git bisect good
6aa51bd26711ac2e7d2d82588c3dd83114e78fc6 is the first bad commit
commit 6aa51bd26711ac2e7d2d82588c3dd83114e78fc6
Author: Ian Leitch <ian@leitch.io>
Date:   Tue Jan 2 12:02:34 2024 +0000

    Detect assign-only properties on structs with synthesized initializers

 CHANGELOG.md                                       |  2 +
 .../Formatters/OutputDeclarationFilter.swift       |  2 +-
 .../AssignOnlyPropertyReferenceEliminator.swift    | 66 +++++-----------------
 .../Mutators/CodingKeyEnumReferenceBuilder.swift   | 11 +---
 .../DefaultConstructorReferenceBuilder.swift       |  4 +-
 ...StructImplicitInitializerReferenceBuilder.swift | 65 +++++++++++++++++++++
 Sources/PeripheryKit/SourceGraph/SourceGraph.swift | 26 ++++++---
 .../SourceGraph/SourceGraphMutatorRunner.swift     |  3 +-
 .../RetentionFixtures/testCodingKeyEnum.swift      | 20 -------
 .../testRetainsEncodableProperties.swift           | 40 -------------
 .../testStructImplicitInitializer.swift            | 18 ++++++
 Tests/PeripheryTests/RetentionTest.swift           | 47 +++++++++------
 12 files changed, 152 insertions(+), 152 deletions(-)
 create mode 100644 Sources/PeripheryKit/SourceGraph/Mutators/StructImplicitInitializerReferenceBuilder.swift
 delete mode 100644 Tests/Fixtures/RetentionFixtures/testRetainsEncodableProperties.swift
 create mode 100644 Tests/Fixtures/RetentionFixtures/testStructImplicitInitializer.swift

I wouldn't take that as gospel, as it's easy to screw up git bisect, and because this is probabilistic, and I did repeated runs, it's possible that I just didn't see the problem on commits that I marked as good.

I've not dug in yet, but my first go around led to this commit.

[mildm8nnered]

So it does appear to be that commit - https://github.com/peripheryapp/periphery/commit/6aa51bd26711ac2e7d2d82588c3dd83114e78fc6

I get completely consistent results on the preceding commit.

The discrepancies I'm seeing with that commit involve the same file as I saw the problem with on 2.18.0, but more variation in what issues are/are not detected. Almost as if the problem was introduced here, but then partly improved, but not completely fixed, in later updates.

[ileitch]

Thanks for investigating 🙇. Is there anything special about the file containing these properties? Can you reproduce the issue with the file extracted into a demo project? A demo project would greatly help me diagnose and fix this issue.

[mildm8nnered]

Thanks for investigating 🙇. Is there anything special about the file containing these properties? Can you reproduce the issue with the file extracted into a demo project? A demo project would greatly help me diagnose and fix this issue.

Nothing special afaik - I have not tried extracting yet, but I will do once I'm back home.

The other thing I was going to try was just reverting the changes in that commit one-by-one to see if I could work out exactly which change caused.

I'll follow up on this later on this week.

[mildm8nnered]

PeripheryDemo.zip

The attached project shows the issue - I've cut the code down as much as possible. All the relevant code is under PeripheryDemoTests - the rest of the project is just a stock new project from the Xcode templates.

I am using Xcode 15.2 on Ventura Intel.

There's a run_periphery_scan.sh script at the top level that invokes the periphery.6aa51bd binary, which is just built from commit 6aa51bd.

I can easily get inconsistent results by doing the following:

% ./run_periphery_scan.sh > w; ./run_periphery_scan.sh > x; ./run_periphery_scan.sh > y; ./run_periphery_scan.sh > z
% cksum w x y z
3310045240 2131 w
2707015723 2552 x
2707015723 2552 y
2707015723 2552 z
%  diff w x
0a1,3
> /Users/martin.redington/Documents/Source/PeripheryDemo/PeripheryDemoTests/DemoTests.swift:12:47: warning: Property 'duration' is unused
> /Users/martin.redington/Documents/Source/PeripheryDemo/PeripheryDemoTests/DemoTests.swift:13:45: warning: Property 'offset' is unused
> /Users/martin.redington/Documents/Source/PeripheryDemo/PeripheryDemoTests/DemoTests.swift:14:47: warning: Property 'periodId' is unused

The allegedly (sometimes) unused vars are here:

private class DemoObject: MockObject {
    static let _mockFields = MockFields()
    typealias MockValueCollectionType = Array<Mock<DemoObject>>

    struct MockFields {
        @Field<Double>("duration") public var duration
        @Field<Double>("offset") public var offset
        @Field<String>("periodId") public var periodId
    }
}

and they are referenced in the convenience initializer immediately afterwards here:

private extension Mock where O == DemoObject {
    convenience init(
        duration: Double? = nil,
        offset: Double? = nil,
        periodId: String? = nil
    ) {
        self.init()
        self.duration = duration
        self.offset = offset
        self.periodId = periodId
    }
}