SwiftLint icon indicating copy to clipboard operation
SwiftLint copied to clipboard

`empty_xctest_method` incorrectly triggering for setUpWithError and tearDownWithError

Open KaneCheshire opened this issue 4 years ago • 5 comments

New Issue Checklist

Describe the bug

Currently on master, the empty_xctest_method rule is incorrectly triggered for overridden setUpWithError and tearDownWithError methods in some cases.

If you update EmptyXCTestMethodRuleExamples.nonTriggeringExamples to include a new non-triggering example with one of the following, the EmptyXCTestMethodRuleTests.testWithDefaultConfiguration unit test will fail:

Example("""
        class TotoTests: XCTestCase {
            var foobar: String = ""

            override func setUpWithError() throws {
                foobar = "anything"
            }
        }
""")

or

Example("""
        class TotoTests: XCTestCase {
            var foobar: String?

            override func tearDownWithError() throws {
                foobar = nil
            }
        }
""")

If we update it to add a (redundant) super call it fixes the issue:

override func setUpWithError() throws {
  try setUpWithError()
  foobar = "anything"
}

However there are two issues with this:

  • This assumes that you have the rule enabled to enforce super calls, which is and should continue to be separate from this rule
  • This assumes that the super call is necessary, which for XCTest setup and tear down functions, it isn't (unless you have your own superclass or something)

Additionally, this has only started failing since updating SwiftLint, we had this rule enabled in 0.38.0 but building the latest version from master and using that causes a failure. I'm not sure exactly what version since 0.38.0 this started happening.

Looking at the EmptyXCTestMethodRule implementation and putting a breakpoint where the StyleViolation is returned, we can see that both enclosedVarParameters and substructure are showing as empty for the functionMethodInstance subDictionary, and that's what's causing the violation.

If I also po dictionary at the same breakpoint, I can't see any way of detecting that the function isn't actually empty. It's like that foobar = "" doesn't exist in the source.

I'm not too savvy with SourceKitten to know if this is a bug with SourceKitten, but since the function isn't actually empty, this is a bit of an issue.

Environment

  • Latest master version as of now
  • Unit test (also works building from source and creating a test case with the same issue and running SwiftLint lint)
  • Xcode 12.5

KaneCheshire avatar Jun 02 '21 10:06 KaneCheshire

I've tried the latest release (0.44.0) but this is still an issue for us 😞

KaneCheshire avatar Sep 15 '21 12:09 KaneCheshire

Looks like an issue with SourceKit according to SourceKitten (see closed issue above), however I'm now trying to understand why it worked with version 0.38.0 of SwiftLint but downloading the latest version has the issue. Surely the toolchain is the same between the two when running in Xcode?

KaneCheshire avatar Sep 17 '21 11:09 KaneCheshire

Perhaps a workaround for now would be to make this more configurable so it doesn't have to impact setup/teardown functions

KaneCheshire avatar Sep 17 '21 11:09 KaneCheshire

I suspect this is related:

empty_xctest_method is triggering on variable functions declared within an XCTestCase class, too.

This triggering code comes straight from the Xcode 13 iOS App template (when “Include tests” is selected):

override class var runsForEachTargetApplicationUIConfiguration: Bool {
    true
}

grantneufeld avatar Oct 01 '21 17:10 grantneufeld

This is still an issue with version 0.47.1 and we've had to either disable the rule or add a call to a function like super.setup() to silence the warnings

ImranRaheem avatar Jul 13 '22 23:07 ImranRaheem

Running into a similar variation of this: image

dimitribouniol avatar Aug 18 '22 17:08 dimitribouniol