[swift-dev] Categorization of warnings in Swift

[Dmitri Gribenko]

On Tue, Jan 12, 2016 at 9:44 AM, Michael Ilseman via swift-dev <
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:
>
>    1.  Categorize based on broad language-feature/compiler-area. E.g.
>    "Availability" or “CommandLineArguments”
>    2.  Categorize based on the kind of warning. E.g. "Deprecated" or
>     “Uninitialized"
>    3.  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.

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.


> 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.

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>*/
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.swift.org/pipermail/swift-dev/attachments/20160112/de61e053/attachment.html>