Skip to content

Add warning when comparing a non-optional value to Optional.none #60893

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

Conversation

calda
Copy link
Contributor

@calda calda commented Sep 1, 2022

This PR ads a warning when comparing a non-optional value to Optional.none.

The compiler already emits a warning when comparing a non-optional value to nil:

let value = "non-optional"

// warning: comparing non-optional value of type 'String' to 'nil' always returns false 
if value == nil {
  print("value is nil")
}

It should also emit a warning when comparing a non-optional value to the Optional.none case:

let value = "non-optional"

// warning: comparing non-optional value of type 'String' to 'Optional.none' always returns false 
if value == .none {
  print("value is nil")
}

// warning: comparing non-optional value of type 'String' to 'Optional.none' always returns false 
if value == Optional.none {
  print("value is nil")
}

// warning: comparing non-optional value of type 'String' to 'Optional.none' always returns false 
if value == String?.none {
  print("value is nil")
}

(In Swift 5.7, these comparisons to nil produce a warning but comparisons to Optional.none do not).

I started a discussion about this on the evolution forums. To me this seems like a bug fix that probably doesn't require an evolution proposal.

@calda calda force-pushed the cal--warn-redundant-comparison-to-optional.none-case branch 2 times, most recently from 70bf088 to f96a003 Compare September 1, 2022 15:36
@calda calda force-pushed the cal--warn-redundant-comparison-to-optional.none-case branch from 535bae1 to c3e74e0 Compare September 1, 2022 16:10
@theblixguy
Copy link
Collaborator

@swift-ci please smoke test

@theblixguy
Copy link
Collaborator

@swift-ci please smoke test

Copy link
Contributor

@xedin xedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@xedin
Copy link
Contributor

xedin commented Sep 9, 2022

@swift-ci please test

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

Successfully merging this pull request may close these issues.

5 participants