From 2c579e9467e2561c9ad38f8c2e6ec0dcd2f383ad Mon Sep 17 00:00:00 2001 From: Ilya Kharlamov <502372+ilyakharlamov@users.noreply.github.com> Date: Wed, 23 Jan 2019 18:22:31 -0500 Subject: [PATCH 1/8] (#215) LCOM4 static fields are now called by FQN --- src/main/java/org/jpeek/skeleton/OpsOf.java | 40 ++++- .../resources/org/jpeek/metrics/LCOM4.xsl | 22 ++- .../resources/org/jpeek/metrics/LCOM5.xsl | 18 +- src/main/resources/org/jpeek/metrics/OCC.xsl | 13 +- src/test/java/org/jpeek/MetricsTest.java | 3 - .../java/org/jpeek/metrics/Lcom4Test.java | 165 ++++++++++++++++++ src/test/java/org/jpeek/metrics/LormTest.java | 76 +++----- .../java/org/jpeek/metrics/MetricBase.java | 150 ++++++++++++++++ .../java/org/jpeek/skeleton/SkeletonTest.java | 4 +- 9 files changed, 424 insertions(+), 67 deletions(-) create mode 100644 src/test/java/org/jpeek/metrics/Lcom4Test.java create mode 100644 src/test/java/org/jpeek/metrics/MetricBase.java diff --git a/src/main/java/org/jpeek/skeleton/OpsOf.java b/src/main/java/org/jpeek/skeleton/OpsOf.java index 9591cb25..5d8240fc 100644 --- a/src/main/java/org/jpeek/skeleton/OpsOf.java +++ b/src/main/java/org/jpeek/skeleton/OpsOf.java @@ -23,6 +23,13 @@ */ package org.jpeek.skeleton; +import org.cactoos.Text; +import org.cactoos.iterable.IterableOf; +import org.cactoos.iterable.Joined; +import org.cactoos.text.JoinedText; +import org.cactoos.text.SplitText; +import org.cactoos.text.TextOf; +import org.cactoos.text.UncheckedText; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; import org.xembly.Directives; @@ -63,16 +70,25 @@ public void visitFieldInsn(final int opcode, final String owner, final String attr, final String dsc) { super.visitFieldInsn(opcode, owner, attr, dsc); this.target.addIf("ops").add("op"); + final String name; if (opcode == Opcodes.GETFIELD) { this.target.attr("code", "get"); + name = attr; } else if (opcode == Opcodes.PUTFIELD) { this.target.attr("code", "put"); + name = attr; } else if (opcode == Opcodes.GETSTATIC) { this.target.attr("code", "get_static"); + name = OpsOf.getQualifiedName(owner, attr); } else if (opcode == Opcodes.PUTSTATIC) { this.target.attr("code", "put_static"); + name = OpsOf.getQualifiedName(owner, attr); + } else { + name = attr; } - this.target.set(attr).up().up(); + this.target.set( + name + ).up().up(); } @Override @@ -87,4 +103,26 @@ public void visitMethodInsn(final int opcode, .up().up(); } + /** + * Returns fully qualified name, an unambiguous name + * that specifies field without regard + * to the context of the call. + * @param owner The class the attribute belongs to + * @param attr The name of the field + * @return Fully qualified name of the field + */ + private static String getQualifiedName(final String owner, + final String attr) { + return new UncheckedText( + new JoinedText( + new TextOf("."), + new Joined( + new SplitText(owner, "/"), + new IterableOf<>( + new TextOf(attr) + ) + ) + ) + ).asString(); + } } diff --git a/src/main/resources/org/jpeek/metrics/LCOM4.xsl b/src/main/resources/org/jpeek/metrics/LCOM4.xsl index 2e2ab20a..40ef65ce 100644 --- a/src/main/resources/org/jpeek/metrics/LCOM4.xsl +++ b/src/main/resources/org/jpeek/metrics/LCOM4.xsl @@ -47,16 +47,23 @@ SOFTWARE. - + + + + + + + + + + + + - + + - diff --git a/src/main/resources/org/jpeek/metrics/LCOM5.xsl b/src/main/resources/org/jpeek/metrics/LCOM5.xsl index 4b77f929..0729fffc 100644 --- a/src/main/resources/org/jpeek/metrics/LCOM5.xsl +++ b/src/main/resources/org/jpeek/metrics/LCOM5.xsl @@ -46,12 +46,24 @@ SOFTWARE. + + + + + + + + + + + + - - + + - + diff --git a/src/main/resources/org/jpeek/metrics/OCC.xsl b/src/main/resources/org/jpeek/metrics/OCC.xsl index 576acfdf..7051d2af 100644 --- a/src/main/resources/org/jpeek/metrics/OCC.xsl +++ b/src/main/resources/org/jpeek/metrics/OCC.xsl @@ -41,7 +41,18 @@ SOFTWARE. - + + + + + + + + + + + + diff --git a/src/test/java/org/jpeek/MetricsTest.java b/src/test/java/org/jpeek/MetricsTest.java index 293b12e5..c9cdfd06 100644 --- a/src/test/java/org/jpeek/MetricsTest.java +++ b/src/test/java/org/jpeek/MetricsTest.java @@ -163,9 +163,6 @@ public static Collection targets() { new Object[] {"OverloadMethods", "TLCOM", 0.0d}, new Object[] {"TwoCommonAttributes", "TLCOM", 4.0d}, new Object[] {"WithoutAttributes", "TLCOM", 1.0d}, - new Object[] {"MethodMethodCalls", "LCOM4", 0.6d}, - new Object[] {"OneCommonAttribute", "LCOM4", 1d}, - new Object[] {"NotCommonAttributes", "LCOM4", 1.5d}, new Object[] {"Bar", "LCC", 0.0d}, new Object[] {"Foo", "LCC", 1.0d}, new Object[] {"MethodMethodCalls", "LCC", 0.1d}, diff --git a/src/test/java/org/jpeek/metrics/Lcom4Test.java b/src/test/java/org/jpeek/metrics/Lcom4Test.java new file mode 100644 index 00000000..7498a34c --- /dev/null +++ b/src/test/java/org/jpeek/metrics/Lcom4Test.java @@ -0,0 +1,165 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2017-2019 Yegor Bugayenko + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ + +package org.jpeek.metrics; + +import org.cactoos.list.ListOf; +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; + +/** + * Test case for LCOM4 + * LCOM4 = Logical Relatedness of Methods. + * Basically it's a graph disjoint set. + * I.e. it answers the the following question: + * "Into how many independent classes can you split this class?" + * @author Ilya Kharlamov (ilya.kharlamov@gmail.com) + * @version $Id$ + * @since 0.28 + * @checkstyle JavadocMethodCheck (500 lines) + * @checkstyle ClassDataAbstractionCouplingCheck (500 lines) + */ +public final class Lcom4Test { + + /** + * Variable from the report. + */ + public static final String PAIRS = "pairs"; + + /** + * Variable from the report. + */ + public static final String ATTRIBUTES = "attributes"; + + /** + * Variable from the report. + */ + public static final String METHODS = "methods"; + + /** + * Test helper. + */ + private MetricBase base; + + @Before + public void setUpMetric() throws Exception { + this.base = new MetricBase( + "org/jpeek/metrics/LCOM4.xsl" + ); + } + + /** + * Tests the deep dependencies methodFour() -> methodTwo() -> methodOne(). + * MethodMethodCalls.java + * - methodOne uses 'num' directly + * - methodTwo uses 'num' indirectly via methodOne + * - methodThree uses 'num' directly + * - methodFour uses 'num' indirectly via methodTwo and methodOne + * - methodFive does not use 'num' (this is an orphan method, ignored) + * Therefore the number of disjoint sets (LCOM4) should be 1 + * since all the methods that use num + * @todo #215:30min LCOM4: Graph algorithm to determine disjoint sets. + * Currently we can only identify the dependencies via only one graph hop + * so we can't trace methodFour that uses 'num' indirectly + * via methodTwo and methodOne. + * This one will be tricky to implement, + * Since graph algorithms in XSLT - that will be a challenge. + */ + @Test + @Ignore + public void methodMethodCalls() { + final MetricBase.Report report = this.base.transform( + "MethodMethodCalls" + ); + report.assertVariable( + Lcom4Test.METHODS, + new ListOf<>( + "methodOne()", + "methodTwo()", + "methodThree()", + "methodFour()", + "methodFive()" + ).size() + ); + report.assertVariable(Lcom4Test.ATTRIBUTES, 1); + report.assertValue(1.0F); + } + + /** + * In OneCommonAttribute.java. + * - methodOne uses variable 'num' + * - methodTwo uses variable 'num' + * So this class only has a single disjoint set AKA (LCOM4) + * This is an ideal LCOM4 value = 1 + */ + @Test + public void oneCommonAttribute() { + final MetricBase.Report report = this.base.transform( + "OneCommonAttribute" + ); + report.assertVariable( + Lcom4Test.METHODS, + new ListOf<>( + "methodOne", + "methodTwo" + ).size() + ); + report.assertVariable(Lcom4Test.ATTRIBUTES, 1); + report.assertVariable(Lcom4Test.PAIRS, 1); + report.assertValue(1.0F); + } + + /** + * In NotCommonAttributes.java + * - methodOne only uses 'num' variable + * - methodTwo only uses 'anotherNum' variable + * So basically there are two separate disjoint sets, + * or two separate classes under the same umbrella. + * So the value is 2.0 + */ + @Test + public void notCommonAttributes() { + final MetricBase.Report report = this.base.transform( + "NotCommonAttributes" + ); + report.assertVariable(Lcom4Test.ATTRIBUTES, 2); + report.assertVariable(Lcom4Test.PAIRS, 0); + report.assertValue(2.0F); + } + + /** + * Should be the same as NotCommonAttributes. + * since constructors are not methods and should be ignored + */ + @Test + public void notCommonAttributesWithAllArgsConstructor() { + final MetricBase.Report report = this.base.transform( + "NotCommonAttributesWithAllArgsConstructor" + ); + report.assertVariable(Lcom4Test.ATTRIBUTES, 2); + report.assertVariable(Lcom4Test.PAIRS, 0); + report.assertValue(2.0F); + } +} diff --git a/src/test/java/org/jpeek/metrics/LormTest.java b/src/test/java/org/jpeek/metrics/LormTest.java index fa7d92ca..e86b68a1 100644 --- a/src/test/java/org/jpeek/metrics/LormTest.java +++ b/src/test/java/org/jpeek/metrics/LormTest.java @@ -24,14 +24,8 @@ package org.jpeek.metrics; -import com.jcabi.xml.XML; -import com.jcabi.xml.XSLDocument; -import org.cactoos.io.ResourceOf; import org.cactoos.list.ListOf; -import org.hamcrest.MatcherAssert; -import org.hamcrest.core.IsEqual; -import org.jpeek.FakeBase; -import org.jpeek.skeleton.Skeleton; +import org.junit.Before; import org.junit.Test; /** @@ -44,54 +38,38 @@ * @checkstyle ClassDataAbstractionCouplingCheck (500 lines) */ public final class LormTest { + /** + * Test helper. + * For easy variable and value assertion. + */ + private MetricBase base; + + @Before + public void setUpMetric() throws Exception { + this.base = new MetricBase( + "org/jpeek/metrics/LORM.xsl" + ); + } @Test public void calculatesVariables() throws Exception { - final XML xml = new XSLDocument( - new ResourceOf( - "org/jpeek/metrics/LORM.xsl" - ).stream() - ).transform( - new Skeleton( - new FakeBase("TwoCommonMethods") - ).xml() + final MetricBase.Report report = this.base.transform( + "TwoCommonMethods" ); final int methods = 6; - MatcherAssert.assertThat( - "N variable 'N' is not calculated correctly", - xml.xpath( - "//class[@id='TwoCommonMethods']/vars/var[@id='N']/text()" - ).get(0), - new IsEqual<>( - String.valueOf(methods) - ) - ); - MatcherAssert.assertThat( - "variable 'R' is not calculated correctly", - xml.xpath( - "//class[@id='TwoCommonMethods']/vars/var[@id='R']/text()" - ).get(0), - new IsEqual<>( - String.valueOf( - new ListOf<>( - "methodTwo -> methodOne", - "methodThree -> methodOne", - "methodFive -> methodFour", - "methodSix -> methodFour" - ).size() - ) - ) + report.assertVariable("N", methods); + report.assertVariable( + "R", + new ListOf<>( + "methodTwo -> methodOne", + "methodThree -> methodOne", + "methodFive -> methodFour", + "methodSix -> methodFour" + ).size() ); - MatcherAssert.assertThat( - "variable 'RN' is not calculated correctly", - xml.xpath( - "//class[@id='TwoCommonMethods']/vars/var[@id='RN']/text()" - ).get(0), - new IsEqual<>( - String.valueOf( - methods * (methods - 1) / 2 - ) - ) + report.assertVariable( + "RN", + methods * (methods - 1) / 2 ); } } diff --git a/src/test/java/org/jpeek/metrics/MetricBase.java b/src/test/java/org/jpeek/metrics/MetricBase.java new file mode 100644 index 00000000..af501cc2 --- /dev/null +++ b/src/test/java/org/jpeek/metrics/MetricBase.java @@ -0,0 +1,150 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2017-2019 Yegor Bugayenko + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package org.jpeek.metrics; + +import com.jcabi.xml.XML; +import com.jcabi.xml.XSLDocument; +import org.cactoos.io.ResourceOf; +import org.hamcrest.MatcherAssert; +import org.hamcrest.core.IsEqual; +import org.hamcrest.number.IsCloseTo; +import org.jpeek.FakeBase; +import org.jpeek.skeleton.Skeleton; + +/** + * Metric test helper. + * @author Ilya Kharlamov (ilya.kharlamov@gmail.com) + * @version $Id$ + * @since 0.28 + */ +public final class MetricBase { + /** + * XSL document. + */ + private final XSLDocument xsl; + + /** + * Ctor. + * @param path Path to the xsl + * @throws Exception If file not found. + */ + public MetricBase(final String path) throws Exception { + this.xsl = new XSLDocument( + new ResourceOf( + path + ).stream() + ); + } + + /** + * Transform a class to assertable xml. + * @param name File name (without an extension) of a class to transform + * @return Xml result of the transformation + */ + public Report transform(final String name) { + return new Report( + name, + this.xsl.transform( + new Skeleton( + new FakeBase(name) + ).xml() + ) + ); + } + + /** + * Assertion helper for xml. + */ + public static final class Report { + /** + * Copy of the transformation xml. + */ + private final XML xml; + + /** + * Class name. + */ + private final String name; + + /** + * Ctor. + * @param name Class name. + * @param xml Resulting xml. + */ + public Report(final String name, final XML xml) { + this.name = name; + this.xml = xml; + } + + /** + * Asserts the variable produced. + * @param variable Variable name + * @param expected Expected value + */ + public void assertVariable(final String variable, final int expected) { + MatcherAssert.assertThat( + String.format( + "Variable '%s' is not calculated correctly for class '%s'", + variable, + this.name + ), + this.xml.xpath( + String.format( + "//class[@id='%s']/vars/var[@id='%s']/text()", + this.name, + variable + ) + ).get(0), + new IsEqual<>( + String.valueOf( + expected + ) + ) + ); + } + + /** + * Asserts the main metric value. + * @param value Expected value of the metric + */ + public void assertValue(final double value) { + MatcherAssert.assertThat( + "The metric value is not calculated properly", + Double.parseDouble( + this.xml.xpath( + String.format( + "//class[@id='%s']/@value", + this.name + ) + ).get(0) + ), + new IsCloseTo( + value, + // @checkstyle MagicNumberCheck (1 line) + 0.001D + ) + ); + } + } +} diff --git a/src/test/java/org/jpeek/skeleton/SkeletonTest.java b/src/test/java/org/jpeek/skeleton/SkeletonTest.java index 2cc95427..246fc704 100644 --- a/src/test/java/org/jpeek/skeleton/SkeletonTest.java +++ b/src/test/java/org/jpeek/skeleton/SkeletonTest.java @@ -27,7 +27,6 @@ import java.io.IOException; import org.hamcrest.MatcherAssert; import org.jpeek.FakeBase; -import org.junit.Ignore; import org.junit.Test; /** @@ -60,7 +59,7 @@ public void createsXml() { "//class[@id='OverloadMethods']/methods[count(method)=5]", "//method[@name='' and @ctor='true']", "//class[@id='Bar']//method[@name='getKey']/ops[count(op)=3]", - "//class[@id='Bar']//method[@name='getKey']/ops/op[@code='put_static' and .='singleton']", + "//class[@id='Bar']//method[@name='getKey']/ops/op[@code='put_static' and .='Bar.singleton']", "//class[@id='Bar']//method[@name='getKey']/ops/op[@code='call' and .='java.lang.String.length']", "//class[@id='Bar']//method[@name='getKey']/ops/op[@code='get' and .='key']", "//class[@id='Bar']//method[@name='']/ops[count(op)=4]" @@ -124,7 +123,6 @@ public void createsOnlyOneMethodIgnoresSynthetic() { } @Test - @Ignore public void findFieldWithQualifiedName() { MatcherAssert.assertThat( XhtmlMatchers.xhtml( From fdbadf6adbca823d2a77e48dfbec1beb134a4a89 Mon Sep 17 00:00:00 2001 From: Ilya Kharlamov <502372+ilyakharlamov@users.noreply.github.com> Date: Thu, 24 Jan 2019 15:03:26 -0500 Subject: [PATCH 2/8] (#215) LCOM4 static fields are now called by FQN (formatting) --- src/main/java/org/jpeek/skeleton/OpsOf.java | 40 ++---------- .../org/jpeek/skeleton/QualifiedName.java | 64 +++++++++++++++++++ .../java/org/jpeek/metrics/Lcom4Test.java | 36 +++++------ .../java/org/jpeek/metrics/MetricBase.java | 18 ++++-- 4 files changed, 99 insertions(+), 59 deletions(-) create mode 100644 src/main/java/org/jpeek/skeleton/QualifiedName.java diff --git a/src/main/java/org/jpeek/skeleton/OpsOf.java b/src/main/java/org/jpeek/skeleton/OpsOf.java index 5d8240fc..895ccbe6 100644 --- a/src/main/java/org/jpeek/skeleton/OpsOf.java +++ b/src/main/java/org/jpeek/skeleton/OpsOf.java @@ -24,12 +24,7 @@ package org.jpeek.skeleton; import org.cactoos.Text; -import org.cactoos.iterable.IterableOf; -import org.cactoos.iterable.Joined; -import org.cactoos.text.JoinedText; -import org.cactoos.text.SplitText; import org.cactoos.text.TextOf; -import org.cactoos.text.UncheckedText; import org.objectweb.asm.MethodVisitor; import org.objectweb.asm.Opcodes; import org.xembly.Directives; @@ -70,21 +65,21 @@ public void visitFieldInsn(final int opcode, final String owner, final String attr, final String dsc) { super.visitFieldInsn(opcode, owner, attr, dsc); this.target.addIf("ops").add("op"); - final String name; + final Text name; if (opcode == Opcodes.GETFIELD) { this.target.attr("code", "get"); - name = attr; + name = new TextOf(attr); } else if (opcode == Opcodes.PUTFIELD) { this.target.attr("code", "put"); - name = attr; + name = new TextOf(attr); } else if (opcode == Opcodes.GETSTATIC) { this.target.attr("code", "get_static"); - name = OpsOf.getQualifiedName(owner, attr); + name = new QualifiedName(owner, attr); } else if (opcode == Opcodes.PUTSTATIC) { this.target.attr("code", "put_static"); - name = OpsOf.getQualifiedName(owner, attr); + name = new QualifiedName(owner, attr); } else { - name = attr; + name = new TextOf(attr); } this.target.set( name @@ -102,27 +97,4 @@ public void visitMethodInsn(final int opcode, .set(owner.replace("/", ".").concat(".").concat(mtd)) .up().up(); } - - /** - * Returns fully qualified name, an unambiguous name - * that specifies field without regard - * to the context of the call. - * @param owner The class the attribute belongs to - * @param attr The name of the field - * @return Fully qualified name of the field - */ - private static String getQualifiedName(final String owner, - final String attr) { - return new UncheckedText( - new JoinedText( - new TextOf("."), - new Joined( - new SplitText(owner, "/"), - new IterableOf<>( - new TextOf(attr) - ) - ) - ) - ).asString(); - } } diff --git a/src/main/java/org/jpeek/skeleton/QualifiedName.java b/src/main/java/org/jpeek/skeleton/QualifiedName.java new file mode 100644 index 00000000..2a457c27 --- /dev/null +++ b/src/main/java/org/jpeek/skeleton/QualifiedName.java @@ -0,0 +1,64 @@ +/** + * The MIT License (MIT) + * + * Copyright (c) 2017-2019 Yegor Bugayenko + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included + * in all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE + * SOFTWARE. + */ +package org.jpeek.skeleton; + +import org.cactoos.Text; +import org.cactoos.iterable.IterableOf; +import org.cactoos.iterable.Joined; +import org.cactoos.text.JoinedText; +import org.cactoos.text.SplitText; +import org.cactoos.text.TextEnvelope; +import org.cactoos.text.TextOf; +import org.cactoos.text.UncheckedText; + +/** + * A fully qualified name of a field, an unambiguous name + * that specifies field without regard + * to the context of the call. + * @author Ilya Kharlamov (ilya.kharlamov@gmail.com) + * @version $Id$ + * @since 0.29 + */ +public final class QualifiedName extends TextEnvelope { + /** + * Ctor. + * @param owner The class the attribute belongs to + * @param attr The name of the field + */ + public QualifiedName(final String owner, final String attr) { + super( + new UncheckedText( + new JoinedText( + new TextOf("."), + new Joined( + new SplitText(owner, "/"), + new IterableOf<>( + new TextOf(attr) + ) + ) + ) + ) + ); + } +} diff --git a/src/test/java/org/jpeek/metrics/Lcom4Test.java b/src/test/java/org/jpeek/metrics/Lcom4Test.java index 7498a34c..392d5769 100644 --- a/src/test/java/org/jpeek/metrics/Lcom4Test.java +++ b/src/test/java/org/jpeek/metrics/Lcom4Test.java @@ -25,7 +25,6 @@ package org.jpeek.metrics; import org.cactoos.list.ListOf; -import org.junit.Before; import org.junit.Ignore; import org.junit.Test; @@ -59,16 +58,9 @@ public final class Lcom4Test { public static final String METHODS = "methods"; /** - * Test helper. + * Path to XSL. */ - private MetricBase base; - - @Before - public void setUpMetric() throws Exception { - this.base = new MetricBase( - "org/jpeek/metrics/LCOM4.xsl" - ); - } + private static final String XSL = "org/jpeek/metrics/LCOM4.xsl"; /** * Tests the deep dependencies methodFour() -> methodTwo() -> methodOne(). @@ -89,8 +81,10 @@ public void setUpMetric() throws Exception { */ @Test @Ignore - public void methodMethodCalls() { - final MetricBase.Report report = this.base.transform( + public void methodMethodCalls() throws Exception { + final MetricBase.Report report = new MetricBase( + Lcom4Test.XSL + ).transform( "MethodMethodCalls" ); report.assertVariable( @@ -115,8 +109,10 @@ public void methodMethodCalls() { * This is an ideal LCOM4 value = 1 */ @Test - public void oneCommonAttribute() { - final MetricBase.Report report = this.base.transform( + public void oneCommonAttribute() throws Exception { + final MetricBase.Report report = new MetricBase( + Lcom4Test.XSL + ).transform( "OneCommonAttribute" ); report.assertVariable( @@ -140,8 +136,10 @@ public void oneCommonAttribute() { * So the value is 2.0 */ @Test - public void notCommonAttributes() { - final MetricBase.Report report = this.base.transform( + public void notCommonAttributes() throws Exception { + final MetricBase.Report report = new MetricBase( + Lcom4Test.XSL + ).transform( "NotCommonAttributes" ); report.assertVariable(Lcom4Test.ATTRIBUTES, 2); @@ -154,8 +152,10 @@ public void notCommonAttributes() { * since constructors are not methods and should be ignored */ @Test - public void notCommonAttributesWithAllArgsConstructor() { - final MetricBase.Report report = this.base.transform( + public void notCommonAttributesWithAllArgsConstructor() throws Exception { + final MetricBase.Report report = new MetricBase( + Lcom4Test.XSL + ).transform( "NotCommonAttributesWithAllArgsConstructor" ); report.assertVariable(Lcom4Test.ATTRIBUTES, 2); diff --git a/src/test/java/org/jpeek/metrics/MetricBase.java b/src/test/java/org/jpeek/metrics/MetricBase.java index af501cc2..a64493d8 100644 --- a/src/test/java/org/jpeek/metrics/MetricBase.java +++ b/src/test/java/org/jpeek/metrics/MetricBase.java @@ -26,6 +26,7 @@ import com.jcabi.xml.XML; import com.jcabi.xml.XSLDocument; import org.cactoos.io.ResourceOf; +import org.cactoos.text.FormattedText; import org.hamcrest.MatcherAssert; import org.hamcrest.core.IsEqual; import org.hamcrest.number.IsCloseTo; @@ -101,14 +102,16 @@ public Report(final String name, final XML xml) { * Asserts the variable produced. * @param variable Variable name * @param expected Expected value + * @throws Exception String format exception */ - public void assertVariable(final String variable, final int expected) { + public void assertVariable(final String variable, + final int expected) throws Exception { MatcherAssert.assertThat( - String.format( + new FormattedText( "Variable '%s' is not calculated correctly for class '%s'", variable, this.name - ), + ).asString(), this.xml.xpath( String.format( "//class[@id='%s']/vars/var[@id='%s']/text()", @@ -127,22 +130,23 @@ public void assertVariable(final String variable, final int expected) { /** * Asserts the main metric value. * @param value Expected value of the metric + * @throws Exception String format exception */ - public void assertValue(final double value) { + public void assertValue(final double value) throws Exception { MatcherAssert.assertThat( "The metric value is not calculated properly", Double.parseDouble( this.xml.xpath( - String.format( + new FormattedText( "//class[@id='%s']/@value", this.name - ) + ).asString() ).get(0) ), new IsCloseTo( value, // @checkstyle MagicNumberCheck (1 line) - 0.001D + 0.001d ) ); } From e36e4547d279fb39b7866fdaad62ccd36fa9cd1e Mon Sep 17 00:00:00 2001 From: Ilya Kharlamov <502372+ilyakharlamov@users.noreply.github.com> Date: Thu, 24 Jan 2019 15:11:25 -0500 Subject: [PATCH 3/8] (#215) LCOM4 static fields are now called by FQN (lorm formatting) --- src/test/java/org/jpeek/metrics/LormTest.java | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/test/java/org/jpeek/metrics/LormTest.java b/src/test/java/org/jpeek/metrics/LormTest.java index e86b68a1..1e2c2be6 100644 --- a/src/test/java/org/jpeek/metrics/LormTest.java +++ b/src/test/java/org/jpeek/metrics/LormTest.java @@ -25,7 +25,6 @@ package org.jpeek.metrics; import org.cactoos.list.ListOf; -import org.junit.Before; import org.junit.Test; /** @@ -38,22 +37,11 @@ * @checkstyle ClassDataAbstractionCouplingCheck (500 lines) */ public final class LormTest { - /** - * Test helper. - * For easy variable and value assertion. - */ - private MetricBase base; - - @Before - public void setUpMetric() throws Exception { - this.base = new MetricBase( - "org/jpeek/metrics/LORM.xsl" - ); - } - @Test public void calculatesVariables() throws Exception { - final MetricBase.Report report = this.base.transform( + final MetricBase.Report report = new MetricBase( + "org/jpeek/metrics/LORM.xsl" + ).transform( "TwoCommonMethods" ); final int methods = 6; From 919851895e4cf2d9959c38d558fd5b0166346264 Mon Sep 17 00:00:00 2001 From: Ilya Kharlamov <502372+ilyakharlamov@users.noreply.github.com> Date: Thu, 24 Jan 2019 22:29:08 -0500 Subject: [PATCH 4/8] (#215) LCOM4 static fields are now called by FQN (more puzzle info) --- src/test/java/org/jpeek/metrics/Lcom4Test.java | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/test/java/org/jpeek/metrics/Lcom4Test.java b/src/test/java/org/jpeek/metrics/Lcom4Test.java index 392d5769..53045851 100644 --- a/src/test/java/org/jpeek/metrics/Lcom4Test.java +++ b/src/test/java/org/jpeek/metrics/Lcom4Test.java @@ -71,13 +71,15 @@ public final class Lcom4Test { * - methodFour uses 'num' indirectly via methodTwo and methodOne * - methodFive does not use 'num' (this is an orphan method, ignored) * Therefore the number of disjoint sets (LCOM4) should be 1 - * since all the methods that use num + * since all the methods use the same num field. * @todo #215:30min LCOM4: Graph algorithm to determine disjoint sets. * Currently we can only identify the dependencies via only one graph hop * so we can't trace methodFour that uses 'num' indirectly * via methodTwo and methodOne. - * This one will be tricky to implement, - * Since graph algorithms in XSLT - that will be a challenge. + * Calculating Disjoint sets (also known as Connected Components), + * requires a graph traversal algorithm like depth-first search. + * This one will be tricky to implement in XSLT. + * After implementing, remove the @Ignore. */ @Test @Ignore From 63e7dc8285ee7ff671186480421c051094e2c594 Mon Sep 17 00:00:00 2001 From: Ilya Kharlamov <502372+ilyakharlamov@users.noreply.github.com> Date: Fri, 25 Jan 2019 09:16:18 -0500 Subject: [PATCH 5/8] (#215) LCOM4 static fields are now called by FQN (+cactoos-matchers 0.13) --- pom.xml | 6 ++++ .../org/jpeek/skeleton/QualifiedName.java | 15 ++------- .../java/org/jpeek/metrics/Lcom4Test.java | 8 ++--- .../java/org/jpeek/metrics/MetricBase.java | 33 ++++++++++--------- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/pom.xml b/pom.xml index 9514a332..7e8cc076 100644 --- a/pom.xml +++ b/pom.xml @@ -182,6 +182,12 @@ SOFTWARE. 9.8.0-5 runtime + + org.llorllale + cactoos-matchers + 0.13 + test + diff --git a/src/main/java/org/jpeek/skeleton/QualifiedName.java b/src/main/java/org/jpeek/skeleton/QualifiedName.java index 2a457c27..3331fa6f 100644 --- a/src/main/java/org/jpeek/skeleton/QualifiedName.java +++ b/src/main/java/org/jpeek/skeleton/QualifiedName.java @@ -23,13 +23,8 @@ */ package org.jpeek.skeleton; -import org.cactoos.Text; -import org.cactoos.iterable.IterableOf; -import org.cactoos.iterable.Joined; import org.cactoos.text.JoinedText; -import org.cactoos.text.SplitText; import org.cactoos.text.TextEnvelope; -import org.cactoos.text.TextOf; import org.cactoos.text.UncheckedText; /** @@ -50,13 +45,9 @@ public QualifiedName(final String owner, final String attr) { super( new UncheckedText( new JoinedText( - new TextOf("."), - new Joined( - new SplitText(owner, "/"), - new IterableOf<>( - new TextOf(attr) - ) - ) + ".", + owner.replace('/', '.'), + attr ) ) ); diff --git a/src/test/java/org/jpeek/metrics/Lcom4Test.java b/src/test/java/org/jpeek/metrics/Lcom4Test.java index 53045851..cd299e0f 100644 --- a/src/test/java/org/jpeek/metrics/Lcom4Test.java +++ b/src/test/java/org/jpeek/metrics/Lcom4Test.java @@ -100,7 +100,7 @@ public void methodMethodCalls() throws Exception { ).size() ); report.assertVariable(Lcom4Test.ATTRIBUTES, 1); - report.assertValue(1.0F); + report.assertValue(1.0f); } /** @@ -126,7 +126,7 @@ public void oneCommonAttribute() throws Exception { ); report.assertVariable(Lcom4Test.ATTRIBUTES, 1); report.assertVariable(Lcom4Test.PAIRS, 1); - report.assertValue(1.0F); + report.assertValue(1.0f); } /** @@ -146,7 +146,7 @@ public void notCommonAttributes() throws Exception { ); report.assertVariable(Lcom4Test.ATTRIBUTES, 2); report.assertVariable(Lcom4Test.PAIRS, 0); - report.assertValue(2.0F); + report.assertValue(2.0f); } /** @@ -162,6 +162,6 @@ public void notCommonAttributesWithAllArgsConstructor() throws Exception { ); report.assertVariable(Lcom4Test.ATTRIBUTES, 2); report.assertVariable(Lcom4Test.PAIRS, 0); - report.assertValue(2.0F); + report.assertValue(2.0f); } } diff --git a/src/test/java/org/jpeek/metrics/MetricBase.java b/src/test/java/org/jpeek/metrics/MetricBase.java index a64493d8..d331d88d 100644 --- a/src/test/java/org/jpeek/metrics/MetricBase.java +++ b/src/test/java/org/jpeek/metrics/MetricBase.java @@ -27,11 +27,12 @@ import com.jcabi.xml.XSLDocument; import org.cactoos.io.ResourceOf; import org.cactoos.text.FormattedText; -import org.hamcrest.MatcherAssert; -import org.hamcrest.core.IsEqual; +import org.cactoos.text.TextOf; import org.hamcrest.number.IsCloseTo; import org.jpeek.FakeBase; import org.jpeek.skeleton.Skeleton; +import org.llorllale.cactoos.matchers.Assertion; +import org.llorllale.cactoos.matchers.TextIs; /** * Metric test helper. @@ -106,25 +107,27 @@ public Report(final String name, final XML xml) { */ public void assertVariable(final String variable, final int expected) throws Exception { - MatcherAssert.assertThat( + new Assertion<>( new FormattedText( "Variable '%s' is not calculated correctly for class '%s'", variable, this.name ).asString(), - this.xml.xpath( - String.format( - "//class[@id='%s']/vars/var[@id='%s']/text()", - this.name, - variable - ) - ).get(0), - new IsEqual<>( + () -> new TextOf( + this.xml.xpath( + new FormattedText( + "//class[@id='%s']/vars/var[@id='%s']/text()", + this.name, + variable + ).asString() + ).get(0) + ), + new TextIs( String.valueOf( expected ) ) - ); + ).affirm(); } /** @@ -133,9 +136,9 @@ public void assertVariable(final String variable, * @throws Exception String format exception */ public void assertValue(final double value) throws Exception { - MatcherAssert.assertThat( + new Assertion<>( "The metric value is not calculated properly", - Double.parseDouble( + () -> Double.parseDouble( this.xml.xpath( new FormattedText( "//class[@id='%s']/@value", @@ -148,7 +151,7 @@ public void assertValue(final double value) throws Exception { // @checkstyle MagicNumberCheck (1 line) 0.001d ) - ); + ).affirm(); } } } From 1712cc734b82ad540df70d505b2fb043641f603e Mon Sep 17 00:00:00 2001 From: Ilya Kharlamov <502372+ilyakharlamov@users.noreply.github.com> Date: Fri, 25 Jan 2019 09:29:30 -0500 Subject: [PATCH 6/8] (#215) LCOM4 static fields are now called by FQN (rounding error as a param) --- src/test/java/org/jpeek/metrics/Lcom4Test.java | 6 +++--- src/test/java/org/jpeek/metrics/MetricBase.java | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/test/java/org/jpeek/metrics/Lcom4Test.java b/src/test/java/org/jpeek/metrics/Lcom4Test.java index cd299e0f..803bacf7 100644 --- a/src/test/java/org/jpeek/metrics/Lcom4Test.java +++ b/src/test/java/org/jpeek/metrics/Lcom4Test.java @@ -100,7 +100,7 @@ public void methodMethodCalls() throws Exception { ).size() ); report.assertVariable(Lcom4Test.ATTRIBUTES, 1); - report.assertValue(1.0f); + report.assertValue(1.0f, 0.001); } /** @@ -126,7 +126,7 @@ public void oneCommonAttribute() throws Exception { ); report.assertVariable(Lcom4Test.ATTRIBUTES, 1); report.assertVariable(Lcom4Test.PAIRS, 1); - report.assertValue(1.0f); + report.assertValue(1.0f, 0.001d); } /** @@ -146,7 +146,7 @@ public void notCommonAttributes() throws Exception { ); report.assertVariable(Lcom4Test.ATTRIBUTES, 2); report.assertVariable(Lcom4Test.PAIRS, 0); - report.assertValue(2.0f); + report.assertValue(2.0f, 0.001); } /** diff --git a/src/test/java/org/jpeek/metrics/MetricBase.java b/src/test/java/org/jpeek/metrics/MetricBase.java index d331d88d..9445429e 100644 --- a/src/test/java/org/jpeek/metrics/MetricBase.java +++ b/src/test/java/org/jpeek/metrics/MetricBase.java @@ -133,9 +133,10 @@ public void assertVariable(final String variable, /** * Asserts the main metric value. * @param value Expected value of the metric + * @param error Rounding tolerance since the metric is float number * @throws Exception String format exception */ - public void assertValue(final double value) throws Exception { + public void assertValue(final double value, double error) { new Assertion<>( "The metric value is not calculated properly", () -> Double.parseDouble( @@ -148,8 +149,7 @@ public void assertValue(final double value) throws Exception { ), new IsCloseTo( value, - // @checkstyle MagicNumberCheck (1 line) - 0.001d + error ) ).affirm(); } From 4ae3d32969d00d08eb2eebf33e4310a33cb8bf25 Mon Sep 17 00:00:00 2001 From: Ilya Kharlamov <502372+ilyakharlamov@users.noreply.github.com> Date: Fri, 25 Jan 2019 09:45:50 -0500 Subject: [PATCH 7/8] (#215) LCOM4 static fields are now called by FQN (rounding error as a param) --- src/test/java/org/jpeek/metrics/Lcom4Test.java | 9 +++++---- src/test/java/org/jpeek/metrics/MetricBase.java | 2 +- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/jpeek/metrics/Lcom4Test.java b/src/test/java/org/jpeek/metrics/Lcom4Test.java index 803bacf7..e9241a28 100644 --- a/src/test/java/org/jpeek/metrics/Lcom4Test.java +++ b/src/test/java/org/jpeek/metrics/Lcom4Test.java @@ -39,6 +39,7 @@ * @since 0.28 * @checkstyle JavadocMethodCheck (500 lines) * @checkstyle ClassDataAbstractionCouplingCheck (500 lines) + * @checkstyle MagicNumberCheck (500 lines) */ public final class Lcom4Test { @@ -100,7 +101,7 @@ public void methodMethodCalls() throws Exception { ).size() ); report.assertVariable(Lcom4Test.ATTRIBUTES, 1); - report.assertValue(1.0f, 0.001); + report.assertValue(1.0f, 0.001f); } /** @@ -126,7 +127,7 @@ public void oneCommonAttribute() throws Exception { ); report.assertVariable(Lcom4Test.ATTRIBUTES, 1); report.assertVariable(Lcom4Test.PAIRS, 1); - report.assertValue(1.0f, 0.001d); + report.assertValue(1.0f, 0.001f); } /** @@ -146,7 +147,7 @@ public void notCommonAttributes() throws Exception { ); report.assertVariable(Lcom4Test.ATTRIBUTES, 2); report.assertVariable(Lcom4Test.PAIRS, 0); - report.assertValue(2.0f, 0.001); + report.assertValue(2.0f, 0.001f); } /** @@ -162,6 +163,6 @@ public void notCommonAttributesWithAllArgsConstructor() throws Exception { ); report.assertVariable(Lcom4Test.ATTRIBUTES, 2); report.assertVariable(Lcom4Test.PAIRS, 0); - report.assertValue(2.0f); + report.assertValue(2.0f, 0.001f); } } diff --git a/src/test/java/org/jpeek/metrics/MetricBase.java b/src/test/java/org/jpeek/metrics/MetricBase.java index 9445429e..c32229ac 100644 --- a/src/test/java/org/jpeek/metrics/MetricBase.java +++ b/src/test/java/org/jpeek/metrics/MetricBase.java @@ -136,7 +136,7 @@ public void assertVariable(final String variable, * @param error Rounding tolerance since the metric is float number * @throws Exception String format exception */ - public void assertValue(final double value, double error) { + public void assertValue(final double value, final double error) { new Assertion<>( "The metric value is not calculated properly", () -> Double.parseDouble( From 65f399d506bb52aedb08f5bf0d1a4475b7a01863 Mon Sep 17 00:00:00 2001 From: Ilya Kharlamov <502372+ilyakharlamov@users.noreply.github.com> Date: Fri, 25 Jan 2019 12:35:32 -0500 Subject: [PATCH 8/8] (#215) LCOM4 static fields are now called by FQN (pdd formatting) --- src/test/java/org/jpeek/metrics/Lcom4Test.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jpeek/metrics/Lcom4Test.java b/src/test/java/org/jpeek/metrics/Lcom4Test.java index e9241a28..cf856e9f 100644 --- a/src/test/java/org/jpeek/metrics/Lcom4Test.java +++ b/src/test/java/org/jpeek/metrics/Lcom4Test.java @@ -73,7 +73,7 @@ public final class Lcom4Test { * - methodFive does not use 'num' (this is an orphan method, ignored) * Therefore the number of disjoint sets (LCOM4) should be 1 * since all the methods use the same num field. - * @todo #215:30min LCOM4: Graph algorithm to determine disjoint sets. + * @todo #215:30min LCOM4: Graph algorithm to determine disjoint sets. * Currently we can only identify the dependencies via only one graph hop * so we can't trace methodFour that uses 'num' indirectly * via methodTwo and methodOne.