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

ImportControl: unable to disallow static import #4284

Closed
romani opened this Issue Apr 23, 2017 · 8 comments

Comments

Projects
None yet
3 participants
@romani
Member

romani commented Apr 23, 2017

http://checkstyle.sourceforge.net/config_imports.html#ImportControl

I failed to disallow import static junit.framework by ImportConrol by our config.
Intelllij inspection detected bad import usage - a02397e#diff-737a741c85550b2b090c4d3605764679

I will provide strict issue description a bit later.

@romani romani added the approved label Apr 23, 2017

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Apr 23, 2017

Member

@romani Going by our UTs, it is possible to disallow a static import.

"6:1: " + getCheckMessage(MSG_DISALLOWED, "java.awt.Button.ABORT"),

One thing that may not be obvious, is that you must include the field/method in the disallow/allow unless you use a regexp for a wildcard on the end.
Examples:


Member

rnveach commented Apr 23, 2017

@romani Going by our UTs, it is possible to disallow a static import.

"6:1: " + getCheckMessage(MSG_DISALLOWED, "java.awt.Button.ABORT"),

One thing that may not be obvious, is that you must include the field/method in the disallow/allow unless you use a regexp for a wildcard on the end.
Examples:


@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Apr 24, 2017

Member

@rnveach ,

it is far from obvious ....
if partly defined import works for simple imports the same should be applicable for statics.

If there is a nuance .... during this issue we need to update xdoc to clearly explain behavior.

Do you agree ?

Member

romani commented Apr 24, 2017

@rnveach ,

it is far from obvious ....
if partly defined import works for simple imports the same should be applicable for statics.

If there is a nuance .... during this issue we need to update xdoc to clearly explain behavior.

Do you agree ?

@rnveach

This comment has been minimized.

Show comment
Hide comment
@rnveach

rnveach Apr 24, 2017

Member

@romani I agree and stated that is was not obvious until I saw the UT in my post.

Member

rnveach commented Apr 24, 2017

@romani I agree and stated that is was not obvious until I saw the UT in my post.

@jochenvdv

This comment has been minimized.

Show comment
Hide comment
@jochenvdv

jochenvdv May 17, 2017

Contributor

This is something I stumbled upon and sort of documented (#3821), but only regarding static inner classes. It seems that this issue pertains to static imports in general and the documentation should reflect this.
Feel free to assign me.

Contributor

jochenvdv commented May 17, 2017

This is something I stumbled upon and sort of documented (#3821), but only regarding static inner classes. It seems that this issue pertains to static imports in general and the documentation should reflect this.
Feel free to assign me.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Oct 26, 2017

Member

@jochenvdv , can I unassign this issue from you, looks like you do not have time to fix it ?

Member

romani commented Oct 26, 2017

@jochenvdv , can I unassign this issue from you, looks like you do not have time to fix it ?

@jochenvdv

This comment has been minimized.

Show comment
Hide comment
@jochenvdv

jochenvdv Oct 26, 2017

Contributor

@romani Sorry I forgot about this issue, I wil fix it tomorrow.

Contributor

jochenvdv commented Oct 26, 2017

@romani Sorry I forgot about this issue, I wil fix it tomorrow.

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Nov 7, 2017

Member

I failed to disallow import static junit.framework by ImportConrol by our config.

It is because of our config is not testing "test" code, attention to "path".

    <module name="ImportControl">
      <property name="file" value="${checkstyle.importcontrol.file}"/>
      <property name="path" value="^.*[\\/]src[\\/]main[\\/].*$"/>
    </module>
Member

romani commented Nov 7, 2017

I failed to disallow import static junit.framework by ImportConrol by our config.

It is because of our config is not testing "test" code, attention to "path".

    <module name="ImportControl">
      <property name="file" value="${checkstyle.importcontrol.file}"/>
      <property name="path" value="^.*[\\/]src[\\/]main[\\/].*$"/>
    </module>

romani added a commit that referenced this issue Nov 9, 2017

@romani

This comment has been minimized.

Show comment
Hide comment
@romani

romani Nov 9, 2017

Member

actual fix of issue it at da2bf59

Member

romani commented Nov 9, 2017

actual fix of issue it at da2bf59

romani added a commit that referenced this issue Nov 9, 2017

@romani romani added the miscellaneous label Nov 9, 2017

@romani romani added this to the 8.5 milestone Nov 9, 2017

@romani romani closed this Nov 9, 2017

timurt added a commit to timurt/checkstyle that referenced this issue Dec 19, 2017

timurt added a commit to timurt/checkstyle that referenced this issue Dec 19, 2017

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