- Notifications
You must be signed in to change notification settings - Fork6
Issue #78: extract: grab property info#84
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This is a limitation of Checkstyle.
Everything is recognized as String if it isn't identified as one of the others. This doesn't mean it is truly String. |
e01cedb
to577d413
Compare@Luolc Until the issues I mentioned above are done, I don't think we can rely fully on the type we get from Checkstyle. Even if we do get all types, we would need more reflection to get enumeration values as they could change between releases or in PR. Did you plan to use the types gathered and how? |
@@ -4,5 +4,5 @@ | |||
"http://www.puppycrawl.com/dtds/suppressions_1_1.dtd"> | |||
<suppressions> | |||
<suppress checks="MultipleStringLiteralsExtended" files=".*[\\/]src[\\/]test[\\/]"/> | |||
<suppress checks="MultipleStringLiteralsExtended" files="[\\/]ExtractInfoGeneratorTest.java$|.*[\\/]src[\\/]test[\\/]"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
If we are not going to fix this for any custom tests, put path and Test name as suppression.
Example:.*extract[\\/]\w+Test\.java
@@ -13,7 +13,7 @@ | |||
<suppress checks="MagicNumber" files=".*[\\/]src[\\/]test[\\/]"/> | |||
<suppress checks="AvoidStaticImport" files=".*[\\/]src[\\/]test[\\/]"/> | |||
<suppress checks="WriteTag" files=".*[\\/]src[\\/]test[\\/]"/> | |||
<suppress checks="MultipleStringLiterals" files=".*[\\/]src[\\/]test[\\/]"/> | |||
<suppress checks="MultipleStringLiterals" files="[\\/]ExtractInfoGeneratorTest.java$|.*[\\/]src[\\/]test[\\/]"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
same.
@@ -27,4 +27,9 @@ | |||
<Class name="~.*\.Immutable.*"/> | |||
<Bug pattern="NP_METHOD_PARAMETER_TIGHTENS_ANNOTATION"/> | |||
</Match> | |||
<Match> | |||
<!-- We can't change generated source code. --> | |||
<Class name="~.*\.GsonAdapters.*"/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Where is this file/class? I can't remember.
private static final Set<String> FILESET_PROPERTIES = getProperties(AbstractFileSetCheck.class); | ||
/** Properties without document. */ | ||
private static final List<String> UNDOCUMENTED_PROPERTIES = Arrays.asList( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
We are not documenting properties here.
This is more a list of fields that aren't properties.
@@ -40,12 +47,57 @@ | |||
* @author LuoLiangchen | |||
*/ | |||
public class ExtractInfoGeneratorTest { | |||
/** Modules which do not have global properties to drop. */ | |||
private static final List<String> XML_FILESET_LIST = Arrays.asList( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Can we drop this and use the check's class hierarchy to determine the parent?
@@ -94,6 +148,116 @@ else if (ModuleReflectionUtils.isRootModule(clazz)) { | |||
object.add("interfaces", interfaces); | |||
object.add("hierarchies", hierarchies); | |||
final JsonUtil.JsonArray properties = new JsonUtil.JsonArray(); | |||
for (String propertyName : getNecessaryProperties(clazz)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Add empty line beforefor
and after}
forfor
.
@Luolc ping |
1 similar comment
@Luolc ping |
Uh oh!
There was an error while loading.Please reload this page.
#78
Processing result:https://gist.github.com/Luolc/241c6e730deff72035abdb8e7feae889
Add property name and type.
Now type could be detected as follows:
Comparing tohttp://checkstyle.sourceforge.net/property_types.html,
Pattern
is the type ofRegexp
. Other enums are all seen as string. I haven't find a proper way to handle all the enums currently.