Skip to content

Pending issues for DeadCodeAnalysis #29519

@mavasani

Description

@mavasani

Design issues that were brought up in #29511:

  • 1. Should we turn off the analyzers for named types which have IInvalidOperation in it's executable code? - Current implementation of this analyzer does this. Should MakeReadOnly also be changed to be consistent?
    Design decision: In general, it is better to have the analyzer ignore unrelated invalid operations, but given this analyzer operates purely on reference analysis, we cannot be sure which invalid operation can or cannot be related to symbol references. We will stick with the current approach of bailing out in presence of syntax/semantic errors.
  • 2. Do we need a separate diagnostic ID and message for two cases handled the PR:
    1. Members with no read or writes: Type '{0}' has an unused private member '{1}' which can be removed.
    2. Only writes: Type '{0}' has a private member '{1}' which can be removed as the value assigned to it is never used.
      Current implementation of this analyzer has two separate rule IDs and messages.
      Design decision: Separate diagnostic IDs as they are different kind of issues.
  • 3. How do we handle the code fix for case ii. above? Current implementation removes the declaration, but does not alter the references as it probably needs user action on whether to retain the assigned value computation or not. Also this is inline with the decision we made a while ago on remove unused parameters code fix. - Current implementation has no code fix for case ii.
    Design decision: We did not discuss this in detail, but the general view is this can be done subsequently based on the user feedback, especially given there will still be large number of cases that cannot be covered. We will not tackle this right now.
  • 4. Handle diagnostic + fix better for cases where member is only used in nameof/typeof/sizeof expressions. See comment https://github.com/dotnet/roslyn/pull/29511/files#r212735506 - This is not relevant if we decide no code fix for 3.
    Design decision: Same conclusion as 3. above. We will not tackle this right now.
  • 5. Tools/Options UI for rule configuration: This analyzer, and especially the rule ii. above without a code fix, is not a code style rule, but a code quality rule. What is the configuration story for such a rule, given we are likely to add more code quality rules in future as well?
    1. Create a new Code Quality page under Text Editor -> C# -> Advanced, which uses the same UI as the code style rules to allow users to configure both severity and preferences for such rules
    2. Create a separate Analysis section in the Text Editor -> C# -> Advanced page, with check boxes for each code quality to rule to enable/disable the rule. Changing severity and/or preferences will need to happen through explicit edits in .editorconfig file.
    3. No configuration UI for such rules. User has to explicitly edit the team's .editorconfig for configuration.
      Design decision: We decided to go with option iii. Non-code style rules that do not need special options, but only severity configuration will be treated in par with third party analyzers/rules. If in future we get feedback about lack of clarity on how these can be configured, we can consider implementing option i. or ii.

Below are the pending follow-up work items for the PR, not design issues:

Metadata

Metadata

Assignees

No one assigned

    Type

    No type
    No fields configured for issues without a type.

    Projects

    Status

    Complete

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions