SonarQube Java
  1. SonarQube Java
  2. SONARJAVA-188

Rule: Class fields visibility should not be public

    Details

    • Type: New Feature New Feature
    • Status: Closed Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.4
    • Component/s: Rules
    • Labels:
      None
    • Number of attachments :
      0

      Description

      This ticket leads to the depreciation of the Checkstyle rule com.puppycrawl.tools.checkstyle.checks.design.VisibilityModifierCheck.

        Issue Links

          Activity

          Hide
          Dinesh Bolkensteyn added a comment -

          Should we ignore public fields with annotations?

          I think that way we will be sure to not generate any false positives.

          A field with some annotation becomes a kind of API.

          I'm thinking about the way SonarQube passes parameters to rules.
          It's done using the @RuleProperty annotation currently.

          It works, whether your field is public or not. It can even be private, it will still work.

          Now, say you write a setter method for it, and make the field private.

          Intuitively, it looks like the only way to set the field is via the setter, and that its logic cannot be bypassed.

          But because of the annotation, SonarQube can find the field and set it via reflection, which is misleading.

          In such a case, I'd rather keep the field public, because it clearly is part of the API.

          That's why we probably need to exclude public fields with annotations.

          Show
          Dinesh Bolkensteyn added a comment - Should we ignore public fields with annotations? I think that way we will be sure to not generate any false positives. A field with some annotation becomes a kind of API. I'm thinking about the way SonarQube passes parameters to rules. It's done using the @RuleProperty annotation currently. It works, whether your field is public or not. It can even be private, it will still work. Now, say you write a setter method for it, and make the field private. Intuitively, it looks like the only way to set the field is via the setter, and that its logic cannot be bypassed. But because of the annotation, SonarQube can find the field and set it via reflection, which is misleading. In such a case, I'd rather keep the field public, because it clearly is part of the API. That's why we probably need to exclude public fields with annotations.
          Hide
          Dinesh Bolkensteyn added a comment -

          ok for that behavior

          Show
          Dinesh Bolkensteyn added a comment - ok for that behavior
          Hide
          Dinesh Bolkensteyn added a comment -

          Done

          Show
          Dinesh Bolkensteyn added a comment - Done
          Hide
          Freddy Mallet added a comment -

          Manually tested !

          Show
          Freddy Mallet added a comment - Manually tested !
          Hide
          Dinesh Bolkensteyn added a comment -

          FYI, I've changed the issue message to make it clear that changing the field to be a static final constant will also get rid off the issue.

          On SonarQube's codebase, this is actually the required action in some cases.

          It's misleading there to only advice to hide it and provide accessors.

          Show
          Dinesh Bolkensteyn added a comment - FYI, I've changed the issue message to make it clear that changing the field to be a static final constant will also get rid off the issue. On SonarQube's codebase, this is actually the required action in some cases. It's misleading there to only advice to hide it and provide accessors.

            People

            • Assignee:
              Dinesh Bolkensteyn
              Reporter:
              Freddy Mallet
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: