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

empty_xctest_method incorrectly triggering for setUpWithError and tearDownWithError #3647

Closed
KaneCheshire opened this issue Jun 2, 2021 · 6 comments · Fixed by #4129
Closed
Assignees
Labels
bug Unexpected and reproducible misbehavior. sourcekit-issue Issues that are caused by a misbehavior in SourceKit and need to be fixed upstream.

Comments

@KaneCheshire
Copy link
Contributor

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
Copy link
Contributor Author

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

@KaneCheshire
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@grantneufeld
Copy link

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
}

@jpsim jpsim added the bug Unexpected and reproducible misbehavior. label Jan 20, 2022
@SimplyDanny SimplyDanny added the sourcekit-issue Issues that are caused by a misbehavior in SourceKit and need to be fixed upstream. label Mar 26, 2022
@ImranRaheem
Copy link

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

@dimitribouniol
Copy link

Running into a similar variation of this:
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected and reproducible misbehavior. sourcekit-issue Issues that are caused by a misbehavior in SourceKit and need to be fixed upstream.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants