New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SuppressCommentFilter not working with ClassDataAbstractionCoupling when specified on class #977

Closed
bananetomate opened this Issue Apr 24, 2015 · 6 comments

Comments

Projects
None yet
3 participants
@bananetomate

bananetomate commented Apr 24, 2015

Version tested: checkstyle 6.5

Configuration example:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE module PUBLIC "-//Puppy Crawl//DTD Check Configuration 1.3//EN" "http://www.puppycrawl.com/dtds/configuration_1_3.dtd">

<module name="Checker">
  <property name="severity" value="warning"/>
  <module name="TreeWalker">
    <property name="tabWidth" value="4"/>
    <module name="FileContentsHolder"/>
    <module name="SuppressWarningsHolder"/>
    <module name="ClassDataAbstractionCoupling"/>
    <module name="MagicNumber"/>
  </module>
  <module name="SuppressWarningsFilter"/>
  <module name="SuppressionCommentFilter">
    <property name="offCommentFormat" value="@cs-\: ([\w\|]+)"/>
    <property name="onCommentFormat" value="@cs\+\: ([\w\|]+)"/>
    <property name="checkFormat" value="$1"/>
  </module>
</module>

Working example with @SuppressWarning
TestClass.java

package com.company.security.model;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import javax.inject.Inject;

import org.apache.commons.lang.Validate;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional
@SuppressWarnings("checkstyle:classdataabstractioncoupling")
@SuppressWarnings("checkstyle:magicnumber")
public class UserService {

    @Inject
    private UserRepository repository;

    @Inject
    private UserPermissionRepository userPermissionRepository;

    @Inject
    private UserRoleRepository userRoleRepository;

    @Inject
    private UserGroupUserRepository userGroupUserRepository;

    private int value = 10022;

    public void doUselessCreations(UserGroup group) {
        new UserByUserGroupEquals(group);
        new UserByRoleEquals(role);
        new UserPermissionByUserEquals(user);
        new UserPermission(user, permission);
        new UserRoleByUserEquals(user);
        new UserRole(TimeBasedIdGenerator.nextId(), user, role);
        new UserGroupUserByUserEquals(user);
        new UserGroupUser(group, user);
        new UserByUsernameEquals(username);
    }
}

Testing

PS C:\dev\Apps\checkstyle> java -jar .\checkstyle-6.5-all.jar -c .\ClassDataAbstractionCoupling-suppression-checkstyle.xml .\TestClass.java
Starting audit...
Audit done.

Non working example with SuppressionCommentFilter
TestClass.java

package com.company.security.model;

import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.List;
import java.util.Set;

import javax.inject.Inject;

import org.apache.commons.lang.Validate;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
@Transactional
// @cs-: ClassDataAbstractionCoupling
// @cs-: MagicNumber
public class UserService {

    @Inject
    private UserRepository repository;

    @Inject
    private UserPermissionRepository userPermissionRepository;

    @Inject
    private UserRoleRepository userRoleRepository;

    @Inject
    private UserGroupUserRepository userGroupUserRepository;

    private int value = 10022;

    public void doUselessCreations(UserGroup group) {
        new UserByUserGroupEquals(group);
        new UserByRoleEquals(role);
        new UserPermissionByUserEquals(user);
        new UserPermission(user, permission);
        new UserRoleByUserEquals(user);
        new UserRole(TimeBasedIdGenerator.nextId(), user, role);
        new UserGroupUserByUserEquals(user);
        new UserGroupUser(group, user);
        new UserByUsernameEquals(username);
    }
}

Testing

PS C:\dev\Apps\checkstyle> java -jar .\checkstyle-6.5-all.jar -c .\ClassDataAbstractionCoupling-suppression-checkstyle.xml .\TestClass.java
Starting audit...
C:\dev\Apps\checkstyle\.\TestClass.java:16:1: warning: Class Data Abstraction Coupling is 9 (max allowed is 7) classes [UserByRoleEquals, UserByUserGr
oupEquals, UserByUsernameEquals, UserGroupUser, UserGroupUserByUserEquals, UserPermission, UserPermissionByUserEquals, UserRole, UserRoleByUserEquals]
.
Audit done.

I noticed that if you put the // @cs-: just below the package declaration it does work. So this might not be a bug but a restriction on how checkstyle works. If so, just close this ticket.

Regards

@bananetomate bananetomate changed the title from SuppressCommentFilter not working with ClassDataAbstractionCoupling to SuppressCommentFilter not working with ClassDataAbstractionCoupling when specified on class Apr 24, 2015

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Apr 24, 2015

Member

SuppressionCommentFilter should be placed right above violation if you put few comments it might not work.

Try to use SuppressWithNearbyCommentFilter, with influenceFormat option.

Member

romani commented Apr 24, 2015

SuppressionCommentFilter should be placed right above violation if you put few comments it might not work.

Try to use SuppressWithNearbyCommentFilter, with influenceFormat option.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Apr 27, 2015

Member

@bananetomate , does workaround work for you ? If yes, please close the issue.

Member

romani commented Apr 27, 2015

@bananetomate , does workaround work for you ? If yes, please close the issue.

@bananetomate

This comment has been minimized.

Show comment
Hide comment
@bananetomate

bananetomate Apr 27, 2015

Yes, we can live with that. Thanks.

Yes, we can live with that. Thanks.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Apr 27, 2015

Member

If smth is missed in dicumentation please request an update, all should be clear and obvious for users

Member

romani commented Apr 27, 2015

If smth is missed in dicumentation please request an update, all should be clear and obvious for users

@bananetomate

This comment has been minimized.

Show comment
Hide comment
@bananetomate

bananetomate Apr 28, 2015

Maybe you could update the documentation to clarify the fact that SuppressCommentFilter and SuppressWithNearbyCommentFilter should be put just above the violation (without other comments in between). Looking at the documentation I don't see the fact that it must be close specified. The fact that 2 @SuppressWarning can be put on the class but 2 // @cs- shouldn't isn't obvious.

Maybe you could update the documentation to clarify the fact that SuppressCommentFilter and SuppressWithNearbyCommentFilter should be put just above the violation (without other comments in between). Looking at the documentation I don't see the fact that it must be close specified. The fact that 2 @SuppressWarning can be put on the class but 2 // @cs- shouldn't isn't obvious.

@MEZk

This comment has been minimized.

Show comment
Hide comment
@MEZk

MEZk Aug 6, 2016

Contributor

@romani

... if you put few comments it might not work.

It will work. However, the comment should be placed before the violation for sure. For example:
SuppressionCommentFilter
SuppressWithNearbyCommentFilter

The problem was happened because the CLASS_DEF starts on the line where the @service annotation is placed. I will update the documentation.

There is an example in the xdoc of how to configure the SuppressWithNearbyCommentFilter check to suppress more than one comment.

Contributor

MEZk commented Aug 6, 2016

@romani

... if you put few comments it might not work.

It will work. However, the comment should be placed before the violation for sure. For example:
SuppressionCommentFilter
SuppressWithNearbyCommentFilter

The problem was happened because the CLASS_DEF starts on the line where the @service annotation is placed. I will update the documentation.

There is an example in the xdoc of how to configure the SuppressWithNearbyCommentFilter check to suppress more than one comment.

MEZk added a commit to MEZk/checkstyle that referenced this issue Aug 6, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Aug 6, 2016

MEZk added a commit to MEZk/checkstyle that referenced this issue Sep 8, 2016

romani added a commit that referenced this issue Sep 8, 2016

@romani romani added this to the 7.2 milestone Sep 8, 2016

@romani romani closed this Sep 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment