[swift-dev] Categorization of warnings in Swift

[Michael Ilseman]

> On Jan 12, 2016, at 1:24 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> 
> On Tue, Jan 12, 2016 at 9:44 AM, Michael Ilseman via swift-dev <swift-dev at swift.org <mailto:swift-dev at swift.org>> wrote:
> Hello, I'm interested in enabling finer-grained control over warning/error
> reporting ala Clang. I've started to put in some infrastructure to
> DiagnosticEngine, and now I'm at the point of determining categorization. 
> 
> I'd like some input (and maybe even some bikeshedding!) on the community's
> thoughts and preferences here. Here's my take on the issue:
> 
> When it comes to defining the categorization, I can see a few approaches: 
>  Categorize based on broad language-feature/compiler-area. E.g. "Availability" or “CommandLineArguments”
>  Categorize based on the kind of warning. E.g. "Deprecated" or  “Uninitialized"
>  Categorize based on severity or specificity of warnings. E.g. "Pedantic" or "UnusedValue" or "NullDereference"
> 
> And, of course, I think that preference should be given to how people would
> actively like to use the categories to control warnings. For example, there's
> a handful of warnings that makes less sense in a REPL or rapid experimentation
> environment, such as variable_never_mutated.
> 
> Since we try to keep warnings on-by-default, the primary usecase will probably be disabling warnings that are somehow inappropriate for the project.
> 

That makes sense.

> Here's a straw-man proposal of some categorization. Of course, one developer's
> annoying pedantic warning is another's life-saving code-smell detector. I am not
> personally tied to these categorizations at all, I'm just interested in there
> being something simple and basic. Below is a list of every warning in Swift with
> an attempt to put it under a category.
> 
> 
> Deprecated:
>     var_not_allowed_in_pattern
>         "Use of '%select{var|let}0' binding here is deprecated and will be "
>         "removed in a future version of Swift"
>     deprecated_c_style_for_stmt
>         "C-style for statement is deprecated and will be removed in a future "
>         "version of Swift"
>     deprecated_convention_attribute
>         "'@%0' attribute is deprecated; '@convention(%1)' should be used "
>         "instead"
>     availability_deprecated
>         "%0 %select{is|%select{is|was}3}1 deprecated"
>         "%select{| %select{on|in}3 %2%select{| %4}3}1"
>     availability_deprecated_msg
>         "%0 %select{is|%select{is|was}3}1 deprecated"
>         "%select{| %select{on|in}3 %2%select{| %4}3}1: %5"
>     availability_deprecated_rename
>         "%0 %select{is|%select{is|was}3}1 deprecated"
>         "%select{| %select{on|in}3 %2%select{| %4}3}1: renamed to '%5'"
>     parameter_curry_syntax_removed
>         "curried function declaration syntax will be removed in a future "
>         "version of Swift; use a single parameter list"
> 
> I think there should be separate categories for deprecated language features and deprecated APIs.

It might also make sense for there to be a DeprecatedTooling or some such for deprecated command line flags, etc. Here’s it broken down some more:

DeprecatedLanguage:
    var_not_allowed_in_pattern
        "Use of '%select{var|let}0' binding here is deprecated and will be "
        "removed in a future version of Swift"
    deprecated_c_style_for_stmt
        "C-style for statement is deprecated and will be removed in a future "
        "version of Swift"
    deprecated_convention_attribute
        "'@%0' attribute is deprecated; '@convention(%1)' should be used "
        “instead"
    parameter_curry_syntax_removed
        "curried function declaration syntax will be removed in a future "
        "version of Swift; use a single parameter list"

DeprecatedAPI:
    availability_deprecated
        "%0 %select{is|%select{is|was}3}1 deprecated"
        "%select{| %select{on|in}3 %2%select{| %4}3}1"
    availability_deprecated_msg
        "%0 %select{is|%select{is|was}3}1 deprecated"
        "%select{| %select{on|in}3 %2%select{| %4}3}1: %5"
    availability_deprecated_rename
        "%0 %select{is|%select{is|was}3}1 deprecated"
        "%select{| %select{on|in}3 %2%select{| %4}3}1: renamed to '%5'"

DeprecatedTooling:
  TBD

>  
> Unsupported:
>     warning_parallel_execution_not_supported
>         "parallel execution not supported; falling back to serial execution"
>     unsupported_synthesize_init_variadic
>         "synthesizing a variadic inherited initializer for subclass %0 is "
>         "unsupported"
> 
> 
> Stylistic/Pedantic/Cleanliness:
>     pbd_never_used_stmtcond
>         "value %0 was defined but never used; consider replacing "
>         "with boolean test"
>     pbd_never_used
>         "initialization of %select{variable|immutable value}1 %0 was never used"
>         "; consider replacing with assignment to '_' or removing it"
>     capture_never_used
>         "capture %0 was never used",
>     variable_never_used
>         "%select{variable|immutable value}1 %0 was never used; "
>         "consider replacing with '_' or removing it"
>     variable_never_mutated
>         "%select{variable|parameter}1 %0 was never mutated; "
>         "consider changing to 'let' constant"
>     variable_never_read
>         "%select{variable|parameter}1 %0 was written to, but never read"
>     expression_unused_result
>         "result of call to %0 is unused"
>     expression_unused_init_result
>         "result of initializer is unused", ())
>     expression_unused_result_message
>         "result of call to %0 is unused: %1"
>     expression_unused_result_nonmutating
>         "result of call to non-mutating function %0 is unused; "
>         "use %1 to mutate in-place"
>     expression_unused_optional_try
>         "result of 'try?' is unused"
>     non_trailing_closure_before_default_args
>         "closure parameter prior to parameters with default arguments will "
>         "not be treated as a trailing closure"
>     parameter_extraneous_double_up
>         "extraneous duplicate parameter name; %0 already has an argument "
>         "label"
>     parameter_extraneous_empty_name
>         "extraneous '_' in parameter: %0 has no keyword argument name"
>     escaped_parameter_name
>         "keyword '%0' does not need to be escaped in argument list"
> 
> 
> CodeSmell/StrongStylisticHints:
>     guard_always_succeeds
>         "'guard' condition is always true, body is unreachable"
>     warn_unqualified_access
>         "use of %0 treated as a reference to %1 in %2 %3"
>     var_pattern_didnt_bind_variables
>         "'%0' pattern has no effect; sub-pattern didn't bind any variables"
>     type_inferred_to_undesirable_type
>         "%select{variable|constant}2 %0 inferred to have type %1, "
>         "which may be unexpected"
>     no_throw_in_try
>         "no calls to throwing functions occur within 'try' expression"
>     no_throw_in_do_with_catch
>         "'catch' block is unreachable because no errors are thrown in 'do' block"
> 
> It seems like some of these can be too pedantic in cross-platform code bases because of #if's that are not analyzable and might need more granular suppression mechanisms.   For example:
> 
> func foo() throws {}
> func bar() {}
> 
> func baz() {
>   do {
> #if os(OSX)
>     try foo()
> #else
>     bar()
> #endif
>     // common code.
>   } catch { // on non-OS X, warning: 'catch' block is unreachable because no errors are thrown in 'do' block
>     // ...
>   }
> }
> 
> I definitely remember we had some similar issues in the standard library or tests, but don't remember the specifics.
> 

Ah, I didn’t think about that. I suppose classification is even more of a gray area than I thought. Do you think that certain ones are more likely than others to come up as a result of platform differences?

no_throw_in_do_with_catch, no_throw_in_try seem like prime examples. How often is it that platform differences yield always-true conditions or unreachable code?


> Dmitri
> 
> -- 
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com <mailto:gribozavr at gmail.com>>*/

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-dev/attachments/20160112/03c680e4/attachment.html>