Vojtech Szocs has posted comments on this change.
Change subject: core: Add no-argument constructor check
......................................................................
Patch Set 1:
(3 comments)
> After rethinking it, I suggest we just drop the entire filtering concept and
> opt for simplicity - EVERYTHING under common or compat should adhere to this
> checkstyle
+1
Filtering Java classes based on their fully-qualified name is far from ideal.
As Allon mentions in his comment, we'd still be vulnerable to things like
MyParms instead of MyParameters. Alternative solution would be to use Java
class metadata (i.e. annotations) to mark classes to check, however this sounds
like an overkill.
I agree to opt for simplicity and drop the filter logic altogether. This will
yield some extra changes, i.e. require *all* classes in "common" module to have
no-arg constructor, but this isn't a bad thing, IMHO.
New patchset coming soon.
PS: since this patch was meant to address missing no-arg constructor issue in
Parameters classes (living in "common" module), for now, I'd suggest to check
only classes in "common" module.
....................................................
File
build-tools-root/ovirt-checkstyle-extension/src/main/java/checks/NoArgConstructorCheck.java
Line 44: }
Line 45:
Line 46: @Override
Line 47: public void visitToken(DetailAST classDef) {
Line 48: if (!isClass(classDef) || !shouldCheckClass(classDef)) {
Agreed, I'll remove the isClass check here.
Line 49: return;
Line 50: }
Line 51:
Line 52: final DetailAST objBlock =
classDef.findFirstToken(TokenTypes.OBJBLOCK);
Line 58: if (child.getType() == TokenTypes.CTOR_DEF) {
Line 59: hasExplicitCtor = true;
Line 60: DetailAST ctorParams =
child.findFirstToken(TokenTypes.PARAMETERS);
Line 61: if (ctorParams.getChildCount() == 0) {
Line 62: hasNoArgCtor = true;
Agreed, returning here sounds better to me, i.e. early return to reduce method
complexity.
Line 63: break;
Line 64: }
Line 65: }
Line 66: child = child.getNextSibling();
Line 77: return classDef.findFirstToken(TokenTypes.LITERAL_CLASS) !=
null;
Line 78: }
Line 79:
Line 80: boolean shouldCheckClass(DetailAST classDef) {
Line 81: return filterPattern != null ?
filterPattern.matcher(getFullyQualifiedClassName(classDef)).matches() : true;
Agreed.
Line 82: }
Line 83:
Line 84: String getFullyQualifiedClassName(DetailAST classDef) {
Line 85: String prefix = resolvePackagePrefix(classDef);
--
To view, visit http://gerrit.ovirt.org/20274
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia4f9a00d6289374b433ed4419552420a3337da50
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Einav Cohen <eco...@redhat.com>
Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches
