Adding a case to an uninhabited enum is liable to break many things. You can choose to define it in a resilient library instead or add a case of your own; these are just two ways in which you can silence the warning and make your code future-proof. By contrast, merely making this warning disappear would not make your code future-proof.
1 Like
The case I just ran into involves the warning "'catch' block is unreachable because no errors are thrown in 'do' block'". Here is the code.
func testInfoRoute() throws {
let expect = self.expectation(description: "get info")
let headers: [String : String] = [HTTPHeaders.authorization: PathTests.authHeader]
performRequest("get", path: "/info", expectation: expect, headers: headers) { response in
XCTAssertEqual(response.statusCode, HTTPStatusCode.OK, "request failed")
var data = Data()
var info: BulkUserInfo?
do {
XCTAssertNoThrow(_ = try response.readAllData(into: &data))
XCTAssertNoThrow(info = try PathTests.app?.settings.decode(BulkUserInfo.self, from: data))
XCTAssertEqual(BaseTest.user?.id, info?.user.id)
} catch {
// do nothing because assertions should have ended
}
expect.fulfill()
}
waitForExpectations(timeout: 1, handler: nil)
}
/// perform request is declared as
func performRequest(_ method: String, path: String, expectation: XCTestExpectation,
headers: [String: String]? = nil,
requestModifier: ((ClientRequest) -> Void)? = nil,
callback: @escaping (ClientResponse) -> Void)
The problem is the unnecessary catch warning with it coded this way. If I take out the catch, I get an error because the callback parameter does not accept a closure that throws. For some reason, the compiler knows that XCTAssertNoThrow prevents a throw as far a do/catch block is concerned, but not for when it comes to deciding if the closure throws.
I'm using Xcode 10.2 and am in no position to upgrade right now. Being able to stop a warning like this would be great. Filling a radar would just invoke a response to try it on the latest version.
I've used clang diagnostic many times in the past and never for deprecated warnings. A global search finds 70+, mostly from 3rd party code, and mostly in unit tests. That's where disabling warnings really makes sense. I'm not rewriting a unit test until it fails.
You can easily avoid such warnings by restructuring your tests.
func testInfoRoute() throws {
let expect = self.expectation(description: "get info")
let headers: [String : String] = [HTTPHeaders.authorization: PathTests.authHeader]
var receivedResponse: ClientResponse!
performRequest("get", path: "/info", expectation: expect, headers: headers) { response in
receivedResponse = response
expect.fulfill()
}
waitForExpectations(timeout: 1)
XCTAssertEqual(receivedResponse.statusCode, HTTPStatusCode.OK, "request failed")
var data = Data()
var info: BulkUserInfo?
XCTAssertNoThrow(_ = try receivedResponse.readAllData(into: &data))
XCTAssertNoThrow(info = try PathTests.app?.settings.decode(BulkUserInfo.self, from: data))
XCTAssertEqual(BaseTest.user?.id, info?.user.id)
}
We're working on releasing our app for Mac Catalyst. This is a challenge as Catalyst uses iOS 13 as min OS level, whereas our main app still supports iOS 11. On Catalyst, we see loads of deprecation warnings.
We're able to work around them all, but it creates a lot of useless code, with the opportunity to introduce new bugs. Just see a few diffs, so you get an idea:
I completely agree that we eventually need to move to the new API, however the best time to do that is when we drop the old OS completely. Introducing branches to use new API that basically does the same thing, when the OS still supports the previous way of doing things is... not very useful.
Note: I am a new user so I can only post one image, however this post relies on quite a few examples via images - you can see & read them on gist:c2260a9631c9d84e6864c0a7f9b21895 Ā· GitHub
Remaining Post follows but misses images:
Sometimes we have code that is not really used on iOS 13 anymore, but still has some references, so we can't deprecate the whole class. This would be all done once we completely remove that (in that case, our legacy theming). However until that's done, I have to write code whose only purpose is to hide a deprecation warning:
We use UIMenuController, and the API changed quite a bit in iOS 13. However until we can drop this from all our products, we either do this or introduce a category that encapsulates this.
There's code that might need more rethinking once we can drop iOS 12, however until that is the case, we branch.
You can argue that this is a Catalyst issue, and it really should have a setting to override the min OS level, however that's unrealistic as well.
The preferred way would be a an annotation to suppress deprecations, just as we have in Objective-C. Swift's goal is to provide an environment where people write safer code in a more expressive way. Not having a way to control deprecations seems counter to this goal.
6 Likes
While deprecations can be annoying to deal with, it seems like being able to disable them, or warnings in general, would be a net negative for the language as a whole. Anyone who's dealt with legacy projects knows what kind of awful mess being able to turn off warnings can leave in a project. Perhaps a better solution would be to treat some types of deprecation as a different type of compile time issue. Swift language deprecations should likely always be warnings, given the importance they can have, but platform and external framework deprecations could be surfaced separately, with a simple compiler flag to ignore them. This would also allow IDEs like Xcode to produce a better UX for these issues, like runtime issues, rather than having to treat them like any other compiler warning.
2 Likes
@Jon_Shier How do you determine that adding such an exception would be a "net negative for the language as a whole"? Any feature can be abused, and folks can write bad code with or without that annotation. Did none of the above examples convince you that this might actually be useful for some cases?
Nobody is forced to use these. But if I have the choice between writing additional code, just to get to zero warnings and annotating some places that we'll revisit later, once it is possible to drop the older API usage, I'd take the annotation. Less risk, less code, easier to search whenever we increase the iOS version. This strategy worked really really well for us in Objective-C for the last 10 years, and we keep these deprecation annotations to a necessary minimum. I don't see any difference why this strategy wouldn't also work great in Swift. Trust the developers.
We equally want to get to zero runtime issues, so converting them wouldn't change the motivation to get to zero.
platform and external framework deprecations could be surfaced separately, with a simple compiler flag to ignore them.
This would be an acceptable solution. I'm not a fan of the all-or-nothing approach, because that will make it harder to find new deprecations, but it's better than the current situation for sure.
16 Likes
I suppose I was speaking more of disabling warnings in general when I said it would be a net negative. Even being able to disable any deprecation warning would be a bad idea, as that would still cause the oft-cited issue of creating dialects of the language, where behaviors and API usage can be vastly different depending on which warnings you have turned on at any given time. This, of course, cascades into different behaviors and bugs depending on the settings, and we saw this all the time in Objective-C.
So even though it's more developer work, supporting newer APIs sooner can lead to better user experiences, whether obvious to the user (view behavior changes), or not (NSURLConnection vs. NSURLSession). As a user, I appreciate the language not allowing developers to ignore the issues without it constantly being in their face. As a developer, this is quite annoying, especially in the case of Catalyst.
A simple solution would be a compiler flag to turn them off until the deployment target meets the availability of the API. That way devs wouldn't be forced to deal with the deprecations immediately, but would have to once they're targeting the OS where the replacement is available.
Similarly, rather than turning them off, the compiler could merely surface deprecation warnings differently than regular warnings, with the same higher priority for deprecations which can now be easily fixed due to a higher deployment target. This would still surface deprecation issues but would let IDEs deal with them more flexibly and allow developers who like to treat warnings as errors to still have clean builds.
A final, more advanced option to turning them off or treating them differently could be a counterpart to #available that can be applied at use sites. Of course, this type of syntax is unprecedented in Swift, so I'm not sure what it would look like, but it could be something like this:
let transitionController = #allowDeprecatedUsage(self.transitionController(forDocumentURL: documentURL))
Ideally, this would still produce a warning if the deployment target was high enough, but it seems like it could be extended to control that more precisely:
let transitionController = #allowDeprecatedUsage(until: iOS 13, self.transitionController(forDocumentURL: documentURL))
Such markup is, of course, rather obnoxiously in your face, but I think that's okay given it shouldn't be used more than absolutely necessary.
This feature would also allow the compiler and IDE integration I mentioned earlier, with additional context when an until value is set.
In the end I suppose this is all my way of saying that, instead of treating deprecation as a warning to turn off (as that sets a bad precedent for the language), it should instead be integrated further into the language for a better experience all around.
1 Like
Even being able to disable any deprecation warning would be a bad idea, as that would still cause the oft-cited issue of creating dialects of the language, where behaviors and API usage can be vastly different depending on which warnings you have turned on at any given time.
This assumes all code are written in front-end apps. Library developers really, really need a way to silence warnings so they don't gets passed above to other modules. Also, silencing errors with a compiler flag would just solve half of the problem. There are times we need the warnings in some places but silenced in others where a workaround has been implemented.
I would argue that having no way to disable warnings is the "net negative". It's a lot more stressful to work on a specific deprecation if there are hundreds of other warnings cluttering the error messages. What happens is that developers tend to ignore warnings more, and more recent warnings get unnoticed.
3 Likes
Iāve yet to see any compelling argument that warnings, aside from deprecation notices, shouldnāt just be fixed immediately. Swift has always tried to ensure that its warnings are high quality and not subjective like so many for C-based languages. At most, what youāre talking about would be something at the package manager level that ignores warnings from dependencies, like you currently can with CocoaPods.
3 Likes
Package manager-level settings nuke all warnings completely, which would be otherwise useful for API consumers (see common use-case posted earlier in the thread). API authors know more about which warnings are safe to ignore, why transfer that responsibility to the thousands of those API's consumers?
I understand the stance of wanting deprecations handled as soon as possible, but what about cases where there is nothing to handle at all? That's the direction I think this discussion should be directed at, we shouldn't let perfection be the enemy of good.
1 Like
I understand the stance of wanting deprecations handled as soon as possible, but what about cases where there is nothing to handle at all? That's the direction I think this discussion should be directed at, we shouldn't let perfection be the enemy of good.
Iām not sure what you mean here. Like I said, I think thereās a case to be made for better deprecation handling, it should just be more nuanced than turning warnings off. But Iād rather the language keep warnings high quality so we can safely say you shouldnāt turn them off than any sort of mechanism to toggle warnings individually.
I think this passage from the diagnostics document in the Swift repo sums up the current philosophy behind warnings nicely:
For a warning, consider whether there might be a legitimate need to write the code in question, even if it's rare. In this case, there should be a way to silence the warning, and a note with a fix-it suggesting this. For example, the "unused function result" warning suggests assigning to
_to make it clear that this is intentional.
If we follow this guideline, then every warning should be able to be silenced by clarifying intent using existing Swift syntax, and we can avoid adding more heavy-weight suppression mechanisms which might be easy to misuse.
Clearly this breaks down a bit in practice, especially in the case of the deprecation warnings discussed upthread. However, I think the best solution may be to tackle these problems on a case by case basis, coming up with a targeted solution specifically for managing deprecation warnings in the source.
I think it's important to consider that out of hundreds of implemented warnings, there are only a small handful which people would currently like to suppress. This suggests to me that the current system of clarifying intent works well except for in a few cases where the language & compiler implementation falls short right now.
14 Likes
Yes, this makes my thinking much more clear. Itās nice to have some high level principles to guide discussion here.
Exactly, I can think of only one type: Deprecations. So it might be worth discussing those separately.
I have worked in two large Swift (iOS) code bases over the last couple of years, both of which had āwarnings as errorsā turned on. And in both deprecations were a huge pain every time we switched to a new SDK.
Those are code-bases were more than 30 developers work on the same app in parallel. Itās simply not feasible for the one person doing the SDK upgrade to fix all the warnings in code they might have never touched before. So that was the main reason in our case for warnings not being fixed immediately.
Another use-case I remember: in one of the projects, I was working on generating Swift networking code based on a Swagger API spec. We would generate a request-class for each endpoint and package all of them up in a framework to be used by feature developers. Now, the Swagger Spec also supported deprecating endpoints which were used when a newer version of the same endpoint was introduced. So we wanted to generate deprecation annotations for the corresponding request classes as well. However, as there is no way to turn them of and the target project had warnings-as-errors turned on, we couldnāt reasonably do this as any update to the generated library and thus any update to the spec by backend devs would have potentially broken the build.
Again, itās not reasonable to expect a dev that updated the API-lib, because they needed access to a new endpoint, to fix all the newly deprecated endpoints in completely different parts of the code base.
So the result here was not āfix deprecations as soon as possibleā but ādonāt surface the deprecations, donāt show them to devs at allā.
In all previous cases my ideal solutions would have been to have a setting for ātreat-warnings-as-errors-except-deprecations-those-should-be-warnings-stillā.
A general āturn of depreciation warningsā setting would not have been very helpful, as that would have just hidden the warnings completely again.
A usage-site code annotation for āI know this usage is deprecated, Iāll fix it later, donāt warnā would have also been very useful, as that would enable the sole updating dev to fix all warnings/errors while still leaving markers in the code for the actual owners of the code to fix.
5 Likes
I agree with the principles and the approach in theory. However...
Yes it does.
Aside from deprecation-related issue, I have had cases where the warning was produced as the result of a compiler bug, or the intended fix was not possible do to a compiler bug. This is really unfortunate as issues like this prevent the use of warnings as errors, something that is greatly preferable to have enabled in CI on large projects.
I'm not sure what the right answer is here, but the current approach definitely feels a bit idealistic, in contrast with Swift's usually very pragmatic approach.
2 Likes
I wonder if there's a middle ground here. E.g. something that only silences warnings for a specific version of the language/compiler.
func doAThing() {
// Silences a warning, but only in Swift 5.1
@silenceWarning(Swift5_1)
makeDeprecatedCall()
}
This allows silencing warnings, but forces users to evaluate the warning when updating. I can see it leading to fear of updating, but it might be a good compromise.
An alternative solution for CI would be to have an expected warnings file, somewhat like the ABI compatibility checks currently used in the Swift standard library. This would allow deliberate introduction of new warnings while still preventing accidental introduction.
1 Like
The downside of this approach is that moves the information about "expected" warnings away from the source code (it's natural home) and it takes a lot more setup that involves a relatively specialized skillset (CI config).
1 Like
The important thing is not the language/compiler version, but the SDK version/minimum deployment target. That's the thing that contains the deprecated function. See this post above:
Agreed. Seems like warnings are generally because you're doing something "code smelly" (like never mutating a var and such), and these concern how the language functions. Deprecation warnings, though, are essentially letting you know that a library maintainer has decided to do something else. Unlike every other warning that I'm aware of, it's not there to point out a technical (syntactic or semantic) issue with your code; it's pointing out that what amounts to a managerial decision in someone else's code will affect your code at some point in the future.
Calling deprecation a "managerial decision" is probably not the best phrasing, but a deprecation warning is pretty clearly a different class of issue than, say, ignoring a function's return value, IMHO.
Hmm... Upon doing a pre-"click the submit button" thread review, I discovered that I already posted in here. I guess this is as much an elaboration of that post as anything else. @jrose, do you still have thoughts on the matter?