Swift should allow for suppression of warnings, especially those that come from Objective-C

Evolution Pitches
1

I have quite a unique problem, and I am canvassing the Swift community in support of a solution.

At my workplace, we have a large amount of Swift. Our apps have several million lines of Swift powering them. We have large numbers of engineers writing Swift too; around 200+ contribute to our iOS codebase. We have so much Swift, that we wonder if we must have the largest Swift codebase around. In talking with other Large Silicon Valley Companies™️, we think we might have the most (and if you're reading this and you have more Swift LoC, I'd love to chat with you to trade notes!)

We also deprecate our oldest supported iOS version every year. We do this around August, because September usually brings a new iOS version to support, and we only want to support three major iOS versions at any given time.

However, because we have lots of Swift, we have one major problem every year when we try to do this which is getting worse and worse every time we do it. Setting the Deployment Target to a newer major version of iOS causes thousands of deprecation–related warnings to appear in our build systems (Xcode, buck, and xcodebuild).

At my employer, we don't like having thousands of warnings cluttering Xcode, so we enable -Werror for our Objective-C and -warnings-as-errors for our Swift. This works well, because engineers writing code always write code that does not surface warnings, and while they are working if there are any new errors or warnings from the compiler then they can easily see them because there aren't thousands of other warnings they didn't create to sift through.

Today, Swift does not provide a way to suppress warnings at the source where those warnings are emitted. But in Objective-C, you could do this:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wdeprecated-declarations"
    [[UIApplication sharedApplication] setStatusBarStyle:UIStatusBarStyleLightContent animated:YES];
#pragma clang diagnostic pop

The warning that is emitted by -Wdeprecated-declarations comes from a deprecation inside Apple's SDK. So if we wrote code that called a deprecated Apple API, we have these options available to us when the warning appears:

  1. Rewrite the code to not emit a warning.
  2. Change the code to call an Objective-C wrapper of Apple's API — the wrapper itself uses the #pragma directives to make sure no warnings are emitted.
  3. Bifurcate our code into two paths; one path that uses the newer API (the one that Apple is telling you to use in the deprecation warning), and one path that uses the older API — by using the @available attribute which means different code paths will be taken at runtime depending on your iOS version.
  4. Disable specific deprecation flags using the -Wno- prefix; e.g., -Wno-deprecated-declarations, and so on.
  5. Disable -warnings-as-errors.

Let me explain why each of these isn't ideal.

1 is obviously the perfect and the ideal solution to the problem. In smaller codebases, this makes a lot of sense. However, our codebase is large. If we have thousands of these warnings, we have to ask many people to take time out of their day to rewrite old code. Because we are risk averse, we will have to wrap the changes inside feature flags (we release our apps on a fixed cadence, e.g., weekly), to make sure that once we roll out the new code if it doesn't work we can still revert to the old code. Materially changing the code has risks, and at least with Objective-C we can look at the warning emitted by Apple and make an informed decision about either changing the code or suppressing the warning. For a lot of the deprecations, we believe that the existing code should continue to work, and if it does not in a future beta of iOS hopefully Apple will notice this too and completely delete it before releasing the new iOS SDK. This doesn't always hold true (and I admit it comes with its own risks), but it is the option we take to minimize issues in production.

2 basically is the approach we follow, taking into account what I said in 1. If we look at a deprecation warning and we decide that we shouldn't risk materially rewriting the code to use a newer API, we write a wrapper in Objective-C of the Apple API. I'm sure you can imagine all the reasons why this isn't great; the most obvious being that we have to duplicate the headerdoc that Apple wrote in the wrapper so that it is surfaced in Xcode, but also that this just adds another hop for our developers when debugging code.

3 is the one I've tried to make work the most, but we have only applied this strategy sparingly for simple cases — the more complex the code, the less likely this can be done easily. But more importantly, this strategy means that you take different paths at runtime based on your iOS version. Obviously as developers we do this all the time in our day–to–day development but that's usually not a big deal if you're writing a new feature and you need to make sure it works with an older Apple API as well as a new one. In this case, you as the developer would know that you support versions x through to y of iOS, therefore if you have to create two code paths using @available you now have to test your code on both paths. But imagine you want to do this to existing code. Just like 1, you have materially changed your code, so you are taking on some risk. You first have to figure out how to get to this code in order to test it. Either you tap away in your app until the code is triggered, or you do some LLDB trickery to run it from the debugger.

4 does not stop engineers from adding new warnings to the codebase for the flags that are disabled. We still want developers to use newer Apple APIs. If we did this, someone could use an API that was deprecated in iOS 5, when our deployment target is currently 9.

5 is the one I consider every year but I can't yet find a way to make this work. I have sometimes pondered if it would be an okay strategy to change the Deployment Target and to disable -warnings-as-errors. But that means that developers will now be able to introduce new warnings into the codebase. Ideally, we could gate potential Pull Requests by enabling -warnings-as-errors in our Continuous Integration but of course because there will be existing warnings in other parts of the codebase, developers would basically always be blocked. I've tried to concoct potential workarounds to this problem but none work in the current setup. One such workaround could be to keep a white list of known warnings in our codebase and to ignore them when builds happen on CI but they would still appear in Xcode. We could possibly make Xcode call our own tools in lieu of Xcode's build system and then drop the warnings before they appear in Xcode, but we want to use the Xcode build system with zero modifications where possible.

So that brings us to proposing a change in Swift. I have wondered about this publicly before (apologies in advance for the crass nature of my tweets):

I feel like I don't really understand the argument against having the ability to suppress warnings in Swift code. I would love to hear some of them from you, the community.

I am very impressed by the way this works in Kotlin, and I would like to propose that if this is resolved in a future Swift evolution proposal, we should take inspiration from their approach.

In Kotlin, you can suppress a given warning on a per–line basis like this:

@Suppress("DEPRECATION") someObject.aDeprecatedFunction()

But you can also do it for an entire function:

@Suppress("DEPRECATION")
fun foo(a: Int,  b: Int) {
  someObject.aDeprecatedFunction()
}

A whole class:

@Suppress("DEPRECATION")
class Baz {
    fun foo(a: Int,  b: Int) {
        someObject.aDeprecatedFunction()
    }
}

Or even a whole file:

@file:@Suppress("DEPRECATION")

Kotlin also has some nice ways to deprecate syntax but that's outside the scope of this proposal.

I'm also sure I might be missing a strategy for this problem. If you have the same problem we do but have a solution that works, I'd love to hear it!

47 Likes
2

I'm a huge +1 on a solution to this problem. The original posts focuses a lot of system API deprecates, but in our codebase we hit this much more often with our own internal public API deprecations.

Sometimes when you want to deprecate an API in our codebase it's not feasible to replace all existing use cases of it. In this case we'd like to use the @available annotation to deprecate the API, but allow the existing cases. As mentioned originally this helps make sure no new warnings are introduced, while still using -warnings-as-errors to make sure developers don't get desensitized to adding warnings.

Currently we "solve" this by marking the API as deprecated with a comment on the class/function/whatever, but on a large team this doesn't really work since developers rarely read them, and other developers who know about the deprecation may not end up reviewing the code that adds new usages of it.

8 Likes
3

This problem is actually two similar but still separate problems:

  • How to handle declaration and use of deprecated APIs that you control
  • How to handle use of deprecated APIs that you don't control

The first issue, which @Keith covers, is one that affects us in swift-protobuf as well, because the protocol buffer spec lets users declare fields as deprecated and we want to reflect that as a deprecation in the generated Swift code as well. But because the field must still be referenced in the generated code (the corresponding property must be touched to read or write the message), it generates unavoidable* warnings. (*without generating much nastier code, like distinguishing the backing storage of the field from the public property)

For that use case, a while back I pitched suppressing deprecation warnings within the same file, which was enough for my use case but not for some others, so the discussion refined it to the idea of attaching an access level to the deprecation that determines where warnings are emitted. I'd like to find the time to revive this, so I'm glad someone brought the topic up again. I have a partial implementation lying around somewhere and it didn't seem too complicated.

For the case of deprecated APIs you don't control, like system APIs, I'm not convinced that we should allow them to be suppressed. Deprecated APIs are deprecated for a reason and shouldn't be relied on; the warning is a constant reminder that the API could disappear or stop functioning correctly later. Developers absolutely should be allocating time for the unglamorous task of migrating when they bump the SDK version, and I see the warnings as an important feature in that regard. Once suppressed, they run the risk of being forgotten, potentially causing more pain later.

Ultimately, I think deprecation should still be left entirely in the hands of the API authors, but we should expand the controls they're given to make it less of an all-or-nothing proposition.

1 Like
4

Thank you for the great post Alan, I think you make the case really well. I'm super +1 on doing something to help here!

11 Likes
5

In a Pretty Large Objective-C Codebase, which will eventually become a Pretty Large Swift Codebase, we approach the same problem from a slightly different angle.

We do seek to migrate away from deprecated APIs when we drop older OS versions. However, the time allocated to get this done across all teams is a couple of months, with several releases expected during that time.

To avoid accumulating random warnings, and also avoid stopping the world, our warning policy starts with -Werror -Wno-error=deprecated – that is, the only permitted warnings are deprecations.

As far as I can see, Swift doesn’t offer this kind of control either, so it looks as if we’ll have to disable -warnings-as-errors during the transition period. This will inevitably lead to a broken window effect as developers ignore non-deprecation warnings that crop up, possibly mitigated by an annoying extra set of checks in CI.

3 Likes
6

I agree this is definitely problem.

Are there any kinds of warnings other than deprecation warnings that have this problem? Perhaps a simple change to allow people to opt deprecation warnings out of warnings-as-errors would be sufficient. Deprecation warnings seem uniquely positioned: there is nothing wrong with the code and most of the time no possibility you made a mistake, it's just communication from the author of the API.

4 Likes
7

Is it generally agreed that warnings and errors are the only kinds of messages compilers need to emit? To me it seems that something like an “advisory memo” might be useful but that also sounds like something that would’ve been debated years (decades?) ago in the general compiler community and I’m just not aware of it.

8

The compiler supports "remarks", but they're not used very much, and I don't think we want to mix them up with warnings or errors. (There are also "notes", of course, which are always supposed to be attached to a parent warning or error.)

I'll try to write up some of my thoughts on warnings, warnings-as-errors, and deprecation warnings specifically in the future.

2 Likes
9

I'm interested in a dialogue around this problem space, for sure.

Issues like potentially not passing flags down to Swift's Clang invocation are clear wins. Transitive deprecation warnings should be improved, although if it requires syntax (does it?) maybe a solution can wait until resilience domains need designing/bikeshedding.

Separately from that, I am very strongly committed against fragmenting Swift with warning flags and/or pragmas. I have never, ever, not once — I'm not being cute, I mean never — witnessed a codebase take up an interest in futzing with warning settings that has not later abused that facility under duress, annoyance, or simply not knowing any better.

Swift warnings and errors should be held to a high standard of describing problems that truly need to be addressed to have a maximally-functioning codebase, in a way that aligns with the language's safety goals. Noting APIs that will stop working in the future as you change your deployment target aligns exactly with that goal.

3 Likes
10

Sure, but at an organizational level it’s useful to distinguish between “must be fixed before you build”, “must be fixed before your push/PR” and “must be fixed before September”.

12 Likes
11

Hi,
Today I wrote some swift that had to deal with obsolete methods. So I put thr obsolete method into a wrapper class, mark that class obsolete for the given iOS version, then create a protocol containing the wrapped methods. Finally, I create an instance of that wrapper class, cast if to the protocol and call the protocol method on it which actually calls the wrapper method which then calls the original. No warnings, all clean code.

Not perfect but works well and groups all obsolete methods neatly in a helper class.

/Andreas Pardeike

1 Like
12

(I am not as opinionated on this as the following might sound, I write this mostly for humour...)

Once warnings have been ignored for a while, how long do you think it will be before we get a request to suppress errors because upgrading introduces too many of them at once?

2 Likes
13

Interestingly you can already subclass deprecated methods without warnings by using this hack:

extension CustomContentsScrollView { // MARK: NSView
    @available(OSX, deprecated: 10.10)
    override public func renewGState() {
        disableScrollTo += 1
        super.renewGState()
        disableScrollTo -= 1
    }
}

I don’t have a good way to call methods that are deprecated, but I agree it’d be super-nice. (Especially since calls like renewGState() provide functionality that is simply not available elsewhere — although in this case it’s special because it’s not what renewGState() does that’s helpful, it’s when it is called by the system.)

-Wil

14

The solution could be an ability to declare your intention about the deprecation at the call site.

If you mark the deprecation as ignored unconditionally, it might haunt you later. Usually there are API replacements available and you’d want to migrate to those at some later date or when you drop support for older SDKs. Some APIs don’t have a replacement yet. For example, SMJobSubmit has been deprecated since macOS 10.10 with (unfulfilled) promise of replacement in libxpc.

We could use something like @ignoreWarnings or @ignoreDeprecation with SDK version argument (akin to @available) to signal that as long as we deploy for this SDK, the deprecation warning should be ignored at this call site. Then once we deploy to a newer SDK version, the warning re-appears and we have either to update the declaration (SDK version in the directive) or to comply with deprecation (by using different API).

In terms of @alanzeino’s original pitch, it would be possible to address point #3 by marking call sites with SDK version corresponding to the version right before when API was introduced. Once you drop all older SDK versions in coming years, the warnings would reappear and you will be able to use only one code path with modern API. And no warnings up until then.

6 Likes
15

I'm of the opinion that one should treat developers as adults. Warnings are emitted by default, so if people want to disable some of them—for whatever reason—they should be able to make that conscious choice. Code review and best practices should make sure that this is not done willy-nilly. (This doesn't preclude additionally implementing ideas, like @pointum suggested, that make it easier to provide more specific conditions to this disabling, but I don't think that it should be a requirement.)

The effect of not being able to turn off some warnings just means that the signal-to-noise ratio will become very bad and in the end, people will just ignore all warnings.

I think this is especially a problem with third-party dependencies for which warnings can't seem to be disabled.

17 Likes
16

i hope this is still up for consideration. we have enabled " -Xfrontend - warn-long-expression-type-checking =400" for our code base, and it is fantastic for helping us improve build times in many places … but sometimes it emits a warning for something that we do not wish to deconstruct further, and which is already pretty simple. being able to disable this particular warning would be super useful on a case-by-case basis and still allow most of our developers to remain at zero warnings while the rest of us work on build-time performance.

4 Likes
17

Hi , I have another use case for silencing warning: unused code path

I'm working with this structure:

public struct Prism<A, B> {
  public let preview: (A) -> B?
  public let review: (B) -> A

  public init(preview: @escaping (A) -> B?, review: @escaping (B) -> A) {
    self.preview = preview
    self.review = review
  }
}

Now, If I define some value for an empty enum:

enum SomeEvent {

// NB : This Event has no case

  static let prism = Prism<AnotherEnum, SomeEvent>(preview: { event in
    guard case let AnotherEnum.someEvent(someEvent) = event else { return nil }
    return someEvent
  }, review: { event -> AnotherEnum in
    // Despite SomeEvent having no cases I'd like to put the implementation here because when it's not this would cause some bugs but I get a warning "will never be executed, SomeEvent is uninhabited"
    return .someEvent(event)
  })
}
18

While the issue of silencing deprecation warnings is important (for reasons noted above), warnings such as what you encounter here can already be silenced by addressing the issue at hand.

Why do you think you need to write code that will never be executed in order to avoid “some bugs”?

2 Likes
19

I think what I'm wanting is for adding additional checks to be easy, and disabling checks to be hard, but not impossible as they are currently.

Thankfully I haven't had to deal with too many unchangeable-legacy code bases, so probably like many, I haven't yet had issues with this in Swift. However, I'm kind of surprised it's not already an option. It seems like one of the basic tools that every language has.

That said, I have worked in legacy Objective-C and C++ where there are hundreds of silenced warnings, (and different sets of warnings depending on the compiler), which has made me wary of the potential effects easily silenced warnings can have. In many ways that's an implementation issue and not something the language should enforce, however I'm wondering if there's a design that encourages properly dealing with warnings rather than silencing being too easy of a fix.

This also reminded me that I would also love ways to more easily opt-in to additional warnings. Ideally there would be a simplified extension point where additional warning/error sets could be added. E.g. if you could integrate SwiftLint directly into the compilation process, rather than having it run as an external tool, or concurrency libraries being able to warn against UI usage on background threads.

My initial thought is to add the ability to silence warnings only via file-specific compiler flags. Adding warnings is probably much more complicated, and isn't something I have experience with. I'm guessing it would require more meta/static level analysis tooling that doesn't currently exist for Swift.

20

In the context of my example, If someone later add a case to the enum, the function would break because it has a return type however if the function returned void the code would compile but with a broken logic. Being able to silence the warning would allow me to write code that would be future proof.