From 359e33a804d4d6e2faf2b56220422a519c911e5f Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Tue, 13 Oct 2020 18:20:33 +0300 Subject: [PATCH 1/3] Making diktat consistent with own code style ### What's done: * Code style --- .../framework/config/TestArgumentsReader.kt | 3 ++ .../test/framework/config/TestConfig.kt | 4 +++ .../test/framework/config/TestConfigReader.kt | 10 ++++++ .../config/TestFrameworkProperties.kt | 17 ++++++++- .../framework/processing/FileComparator.kt | 11 +++--- .../framework/processing/TestCheckWarn.kt | 10 ++++-- .../processing/TestComparatorUnit.kt | 22 +++++++++--- .../test/framework/processing/TestCompare.kt | 36 +++++++++++++++---- .../test/framework/processing/TestMixed.kt | 14 ++++++-- .../processing/TestProcessingFactory.kt | 26 +++++++++----- 10 files changed, 123 insertions(+), 30 deletions(-) diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt index c4ead682ac..f3bea7c350 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt @@ -31,6 +31,9 @@ class TestArgumentsReader( private val cliArguments: List? = readResource(properties.testFrameworkArgsRelativePath) private val cmd: CommandLine + /** + * List of tests provided by user + */ val tests: List get() { val tests = cmd.getOptionValue("t") diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfig.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfig.kt index 4fb098ecb3..f54a7bd6be 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfig.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfig.kt @@ -34,6 +34,10 @@ class TestConfig @JsonCreator internal constructor( """(executionCommand: $executionCommand, expectedResultFile: $expectedResultFile, inPlace: $inPlace, executionType: $executionCommand)""" + /** + * @param testName + * @return [TestConfig] with updated [testName] + */ fun setTestName(testName: String): TestConfig { this.testName = testName return this diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt index 8e5d5ed492..594b3f047c 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt @@ -6,9 +6,19 @@ import java.io.IOException import java.util.stream.Collectors import org.cqfn.diktat.common.config.reader.JsonResourceConfigReader +/** + * A [JsonResourceConfigReader] to read tests configuration as [TestConfig] + */ class TestConfigReader(configFilePath: String, override val classLoader: ClassLoader) : JsonResourceConfigReader() { + /** + * The [TestConfig] which is read from + */ val config: TestConfig? = readResource(configFilePath) + /** + * @param fileStream input stream of data from config file + * @return [TestConfig] read from file + */ @Throws(IOException::class) override fun parseResource(fileStream: BufferedReader): TestConfig { val jsonValue = fileStream.lines().collect(Collectors.joining()) diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestFrameworkProperties.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestFrameworkProperties.kt index 7dbe65d560..e5ca73808c 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestFrameworkProperties.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestFrameworkProperties.kt @@ -2,22 +2,37 @@ package org.cqfn.diktat.test.framework.config import org.cqfn.diktat.common.config.reader.ApplicationProperties -class TestFrameworkProperties(propertiesFileName: String) : ApplicationProperties(propertiesFileName!!) { +/** + * [ApplicationProperties] for running tests + */ +class TestFrameworkProperties(propertiesFileName: String) : ApplicationProperties(propertiesFileName) { private val testFrameworkResourcePath: String get() = properties.getProperty("test.framework.dir") private val testFilesDir: String get() = properties.getProperty("test.files.dir") + /** + * Relative path to a file with arguments for tests runner + */ val testFrameworkArgsRelativePath: String get() = testFrameworkResourcePath + "/" + properties.getProperty("test.framework.arguments") + /** + * Relative path to test files directory + */ val testFilesRelativePath: String get() = "$testFrameworkResourcePath/$testFilesDir" + /** + * Relative path to test configs directory + */ val testConfigsRelativePath: String get() = testFrameworkResourcePath + "/" + properties.getProperty("test.configs.dir") + /** + * Whether tests should be run in parallel + */ val isParallelMode: Boolean get() = java.lang.Boolean.getBoolean(properties.getProperty("parallel.mode")) } diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/FileComparator.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/FileComparator.kt index 0d80c65620..af673ac66a 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/FileComparator.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/FileComparator.kt @@ -1,7 +1,6 @@ package org.cqfn.diktat.test.framework.processing import com.github.difflib.DiffUtils -import org.slf4j.LoggerFactory import java.io.File import java.io.IOException import java.nio.file.Files @@ -9,7 +8,11 @@ import java.nio.file.Paths import java.util.ArrayList import java.util.StringJoiner import java.util.stream.Collectors +import org.slf4j.LoggerFactory +/** + * A class that is capable of comparing files content + */ class FileComparator { private val expectedResultFile: File private val actualResultList: List @@ -19,7 +22,7 @@ class FileComparator { this.actualResultList = actualResultList } - constructor (expectedResultFile: File, actualResultFile: File) { + constructor(expectedResultFile: File, actualResultFile: File) { this.expectedResultFile = expectedResultFile this.actualResultList = readFile(actualResultFile.absolutePath) } @@ -48,15 +51,15 @@ class FileComparator { log.error("""Expected result from <${expectedResultFile.name}> and actual formatted are different. | See difference below: | Expected vs Actual ${System.lineSeparator()}$deltasJoiner""".trimMargin()) - } catch (e: RuntimeException) { - log.error("Not able to prepare diffs between <${expectedResultFile.name}> and <${actualResultList}>", e) + log.error("Not able to prepare diffs between <${expectedResultFile.name}> and <$actualResultList>", e) } return false } /** * @param fileName - file where to write these list to, separated with newlines + * @return a list of lines from the file */ fun readFile(fileName: String): List { var list: List = ArrayList() diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCheckWarn.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCheckWarn.kt index 26e2c5a3cb..392808ba7c 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCheckWarn.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCheckWarn.kt @@ -1,12 +1,18 @@ package org.cqfn.diktat.test.framework.processing +import org.cqfn.diktat.test.framework.config.TestConfig import org.slf4j.Logger import org.slf4j.LoggerFactory -import org.cqfn.diktat.test.framework.config.TestConfig +/** + * [TestCompare] that uses stderr as tests output stream + */ class TestCheckWarn : TestCompare() { - override val log: Logger = LoggerFactory.getLogger(TestCheckWarn::class.java) + @Suppress("MISSING_KDOC_CLASS_ELEMENTS") override val log: Logger = LoggerFactory.getLogger(TestCheckWarn::class.java) private var testConfig: TestConfig? = null + /** + * Get tests execution result + */ override fun getExecutionResult() = testResult.stdErr } diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestComparatorUnit.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestComparatorUnit.kt index 968cc5679a..0de9c19cbd 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestComparatorUnit.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestComparatorUnit.kt @@ -1,18 +1,28 @@ package org.cqfn.diktat.test.framework.processing -import org.apache.commons.io.FileUtils -import org.slf4j.Logger -import org.slf4j.LoggerFactory import java.io.File import java.io.IOException import java.nio.file.Files import java.nio.file.Paths import java.util.ArrayList import java.util.stream.Collectors +import org.apache.commons.io.FileUtils +import org.slf4j.Logger +import org.slf4j.LoggerFactory -@Suppress("ForbiddenComment") +/** + * Class that can apply transformation to an input file and then compare with expected result and output difference. + * @property function a transformation that will be applied to the file + */ +@Suppress("ForbiddenComment", "TYPE_ALIAS") class TestComparatorUnit(private val resourceFilePath: String, private val function: (expectedText: String, testFilePath: String) -> String) { + /** + * @param expectedResult + * @param testFileStr + * @return true if transformed file equals expected result, false otherwise + */ + @Suppress("FUNCTION_BOOLEAN_PREFIX") fun compareFilesFromResources(expectedResult: String, testFileStr: String): Boolean { val expectedPath = javaClass.classLoader.getResource("$resourceFilePath/$expectedResult") val testPath = javaClass.classLoader.getResource("$resourceFilePath/$testFileStr") @@ -36,6 +46,10 @@ class TestComparatorUnit(private val resourceFilePath: String, return FileComparator(expectedFile, actualResult.split("\n")).compareFilesEqual() } + /** + * @param fileName + * @return file content as a list of lines + */ fun readFile(fileName: String): List { var list: List = ArrayList() try { diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCompare.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCompare.kt index c91d2fc440..c50928f123 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCompare.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestCompare.kt @@ -1,5 +1,6 @@ package org.cqfn.diktat.test.framework.processing +import java.io.File import org.apache.commons.io.FileUtils import org.cqfn.diktat.test.framework.common.ExecutionResult import org.cqfn.diktat.test.framework.common.TestBase @@ -7,18 +8,26 @@ import org.cqfn.diktat.test.framework.config.TestConfig import org.cqfn.diktat.test.framework.config.TestFrameworkProperties import org.slf4j.Logger import org.slf4j.LoggerFactory -import java.io.File +/** + * A class that runs tests and compares output with expected result + */ @Suppress("ForbiddenComment") open class TestCompare : TestBase { - protected open val log: Logger = LoggerFactory.getLogger(TestCompare::class.java) + @Suppress("MISSING_KDOC_CLASS_ELEMENTS") protected open val log: Logger = LoggerFactory.getLogger(TestCompare::class.java) private lateinit var expectedResult: File // testResultFile will be used if and only if --in-place option will be used private lateinit var testFile: File private lateinit var testConfig: TestConfig + + /** Result of the test run */ protected lateinit var testResult: ExecutionResult + /** + * @return true if test has passed successfully, false otherwise + */ + @Suppress("FUNCTION_BOOLEAN_PREFIX") override fun runTest(): Boolean { // FixMe: this is an execution for Windows, should support other OS val testPassed = if (testConfig.inPlace) processInPlace() else processToStdOut() @@ -32,6 +41,13 @@ open class TestCompare : TestBase { return testPassed } + /** + * injects test configuration that was read from .json config file + * + * @param testConfig json configuration + * @param properties config from properties + * @return test instance itself + */ override fun initTestProcessor(testConfig: TestConfig, properties: TestFrameworkProperties): TestCompare { this.testConfig = testConfig this.expectedResult = buildFullPathToResource( @@ -43,10 +59,12 @@ open class TestCompare : TestBase { return this } + // STRING_TEMPLATE_CURLY_BRACES is disabled temporarily, until https://github.com/cqfn/diKTat/issues/401 is fixed + @Suppress("STRING_TEMPLATE_CURLY_BRACES", "FUNCTION_BOOLEAN_PREFIX") private fun processInPlace(): Boolean { val copyTestFile = File("${testFile}_copy") FileUtils.copyFile(testFile, copyTestFile) - executeCommand("cmd /c " + testConfig.executionCommand + " " + copyTestFile) + executeCommand("cmd /c ${testConfig.executionCommand} $copyTestFile") val testPassed = FileComparator(expectedResult, copyTestFile).compareFilesEqual() FileUtils.forceDelete(copyTestFile) @@ -54,17 +72,21 @@ open class TestCompare : TestBase { return testPassed } + @Suppress("FUNCTION_BOOLEAN_PREFIX") private fun processToStdOut(): Boolean { - this.testResult = executeCommand("cmd /c " + testConfig.executionCommand + " " + testFile) + this.testResult = executeCommand("cmd /c ${testConfig.executionCommand} $testFile") return FileComparator(expectedResult, getExecutionResult()).compareFilesEqual() } private fun buildFullPathToResource(resourceFile: String, resourceAbsolutePath: String): File { - val fileURL = javaClass.classLoader.getResource("$resourceAbsolutePath/$resourceFile") - require(fileURL != null) { "Cannot read resource file $$resourceAbsolutePath/$resourceFile - it cannot be found in resources" } - return File(fileURL.file) + val fileUrl = javaClass.classLoader.getResource("$resourceAbsolutePath/$resourceFile") + require(fileUrl != null) { "Cannot read resource file $$resourceAbsolutePath/$resourceFile - it cannot be found in resources" } + return File(fileUrl.file) } + /** + * Get result of the test execution + */ protected open fun getExecutionResult() = testResult.stdOut } diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestMixed.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestMixed.kt index d71c283deb..e8d84178ce 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestMixed.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestMixed.kt @@ -4,12 +4,20 @@ import org.cqfn.diktat.test.framework.common.TestBase import org.cqfn.diktat.test.framework.config.TestConfig import org.cqfn.diktat.test.framework.config.TestFrameworkProperties +@Suppress("MISSING_KDOC_TOP_LEVEL", "KDOC_NO_EMPTY_TAGS") // fixme: add documentation when implementation is done class TestMixed : TestBase { private lateinit var testConfig: TestConfig - override fun runTest(): Boolean { - return true - } + /** + * @return + */ + override fun runTest() = true + + /** + * @param testConfig + * @param properties + * @return + */ override fun initTestProcessor(testConfig: TestConfig, properties: TestFrameworkProperties): TestMixed { this.testConfig = testConfig return this diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt index d1537d5a48..0c67ba5fa1 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/processing/TestProcessingFactory.kt @@ -1,17 +1,20 @@ package org.cqfn.diktat.test.framework.processing -import org.slf4j.LoggerFactory -import org.cqfn.diktat.test.framework.common.TestBase -import org.cqfn.diktat.test.framework.config.TestArgumentsReader -import org.cqfn.diktat.test.framework.config.TestConfig -import org.cqfn.diktat.test.framework.config.TestConfig.ExecutionType -import org.cqfn.diktat.test.framework.config.TestConfigReader import java.io.File import java.io.IOException import java.util.concurrent.atomic.AtomicInteger import java.util.stream.Stream import kotlin.system.exitProcess +import org.cqfn.diktat.test.framework.common.TestBase +import org.cqfn.diktat.test.framework.config.TestArgumentsReader +import org.cqfn.diktat.test.framework.config.TestConfig +import org.cqfn.diktat.test.framework.config.TestConfig.ExecutionType +import org.cqfn.diktat.test.framework.config.TestConfigReader +import org.slf4j.LoggerFactory +/** + * A class that runs tests based on configuration + */ @Suppress("ForbiddenComment") class TestProcessingFactory(private val argReader: TestArgumentsReader) { private val allTestsFromResources: List @@ -24,7 +27,8 @@ class TestProcessingFactory(private val argReader: TestArgumentsReader) { } val resource = File(fileUrl.file) try { - return resource.walk() + return resource + .walk() .filter { file -> file.isFile } .map { file -> file.name.replace(".json", "") } .toList() @@ -34,6 +38,9 @@ class TestProcessingFactory(private val argReader: TestArgumentsReader) { } } + /** + * Run all tests specified in input parameters and log results + */ fun processTests() { val failedTests = AtomicInteger(0) val passedTests = AtomicInteger(0) @@ -48,14 +55,15 @@ class TestProcessingFactory(private val argReader: TestArgumentsReader) { val testStream: Stream = if (argReader.properties.isParallelMode) testList.parallelStream() else testList.stream() - testStream.map { test: String -> findTestInResources(test) } + testStream + .map { test: String -> findTestInResources(test) } .filter { it != null } .map { it as TestConfig } .forEach { test: TestConfig -> if (processTest(test)) passedTests.incrementAndGet() else failedTests.incrementAndGet() } - log.info("Test processing finished. Passed tests: [${passedTests}]. Failed tests: [${failedTests}]") + log.info("Test processing finished. Passed tests: [$passedTests]. Failed tests: [$failedTests]") } private fun findTestInResources(test: String): TestConfig? = From 31d1d8556c861bac12050f407a7627e25630d14b Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Thu, 15 Oct 2020 11:32:21 +0300 Subject: [PATCH 2/3] Making diktat consistent with own code style ### What's done: * Code style --- diktat-analysis.yml | 3 ++- .../org/cqfn/diktat/common/cli/CliArgument.kt | 10 ++++---- .../common/config/rules/RulesConfigReader.kt | 15 +++++++----- .../test/framework/common/ExecutionResult.kt | 2 ++ .../framework/config/TestArgumentsReader.kt | 4 ++-- .../test/framework/config/TestConfig.kt | 23 ++++++++++--------- .../test/framework/config/TestConfigReader.kt | 5 ++-- 7 files changed, 35 insertions(+), 27 deletions(-) diff --git a/diktat-analysis.yml b/diktat-analysis.yml index fb7cc63766..0591b7ff8a 100644 --- a/diktat-analysis.yml +++ b/diktat-analysis.yml @@ -146,7 +146,8 @@ configuration: {} - name: FILE_WILDCARD_IMPORTS enabled: true - configuration: {} + configuration: + allowedWildcards: "kotlinx.serialization.*" - name: NO_BRACES_IN_CONDITIONALS_AND_LOOPS enabled: true configuration: {} diff --git a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/cli/CliArgument.kt b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/cli/CliArgument.kt index 14ab36b09b..70bb5a2e47 100644 --- a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/cli/CliArgument.kt +++ b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/cli/CliArgument.kt @@ -1,20 +1,20 @@ package org.cqfn.diktat.common.cli -import org.apache.commons.cli.Option import kotlinx.serialization.* +import org.apache.commons.cli.Option /** * This class is used to serialize/deserialize json representation * that is used to store command line arguments + * @property shortName short argument representation like -h + * @property longName long argument representation like --help + * @property hasArgs indicates if option should have explicit argument */ @Serializable -data class CliArgument ( - // short argument representation like -h +data class CliArgument( private val shortName: String, private val helpDescr: String, - // long argument representation like --help private val longName: String, - // indicates if option should have explicit argument private val hasArgs: Boolean, private val isRequired: Boolean) { /** diff --git a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt index f648215a11..e3462ec60e 100644 --- a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt +++ b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt @@ -6,10 +6,10 @@ package org.cqfn.diktat.common.config.rules import com.charleskorn.kaml.Yaml import com.charleskorn.kaml.YamlConfiguration -import kotlinx.serialization.Serializable -import kotlinx.serialization.decodeFromString import java.io.BufferedReader import java.io.File +import kotlinx.serialization.Serializable +import kotlinx.serialization.decodeFromString import org.cqfn.diktat.common.config.reader.JsonResourceConfigReader import org.slf4j.Logger import org.slf4j.LoggerFactory @@ -28,6 +28,9 @@ interface Rule { /** * Configuration of individual [Rule] + * @property name name of the rule + * @property enabled + * @property configuration a map of strings with configuration options */ @Serializable data class RulesConfig( @@ -38,12 +41,14 @@ data class RulesConfig( /** * Configuration that allows customizing additional options of particular rules. + * @property config a map of strings with configuration options for a particular rule */ open class RuleConfiguration(protected val config: Map) object EmptyConfiguration : RuleConfiguration(mapOf()) /** * class returns the list of configurations that we have read from a yml: diktat-analysis.yml + * @property classLoader a [ClassLoader] used to load configuration file */ open class RulesConfigReader(override val classLoader: ClassLoader) : JsonResourceConfigReader>() { private val yamlSerializer by lazy { Yaml(configuration = YamlConfiguration(strictMode = true)) } @@ -54,10 +59,8 @@ open class RulesConfigReader(override val classLoader: ClassLoader) : JsonResour * @param fileStream a [BufferedReader] representing loaded rules config file * @return list of [RulesConfig] */ - override fun parseResource(fileStream: BufferedReader): List { - return fileStream.use { stream -> - yamlSerializer.decodeFromString(stream.readLines().joinToString(separator = "\n")) - } + override fun parseResource(fileStream: BufferedReader) = fileStream.use { stream -> + yamlSerializer.decodeFromString(stream.readLines().joinToString(separator = "\n")) } /** diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/common/ExecutionResult.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/common/ExecutionResult.kt index f78ac9b717..cc44b55b39 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/common/ExecutionResult.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/common/ExecutionResult.kt @@ -5,5 +5,7 @@ package org.cqfn.diktat.test.framework.common * * @param stdOut standard output * @param stdErr error stream + * @property stdOut content from stdout stream + * @property stdErr content from stderr stream */ data class ExecutionResult(val stdOut: List, val stdErr: List) diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt index ee477b06c0..e5ae0cbe22 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestArgumentsReader.kt @@ -1,11 +1,11 @@ package org.cqfn.diktat.test.framework.config -import kotlinx.serialization.decodeFromString -import kotlinx.serialization.json.Json import java.io.BufferedReader import java.io.IOException import java.util.stream.Collectors import kotlin.system.exitProcess +import kotlinx.serialization.decodeFromString +import kotlinx.serialization.json.Json import org.apache.commons.cli.CommandLine import org.apache.commons.cli.CommandLineParser import org.apache.commons.cli.DefaultParser diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfig.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfig.kt index 3aa801b2d0..a8b12159cf 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfig.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfig.kt @@ -6,21 +6,22 @@ import kotlinx.serialization.Serializable /** * This class is used to serialize/deserialize json representation * that is used to store command line arguments + * @property executionCommand command line execution command, use shell like "cmd", "bash" or other + * @property expectedResultFile expected result file can be a full path or a relative path to a resource + * @property testFile testFile can be a full path or a relative path to a resource + * @property executionType executionType that controls processing of the test (like COMPARE, MIXED, CHECK_WARN, e.t.c) + * @property testProfile + * @property inPlace option that controls if changes and automatic fix should be done directly in file */ @Serializable @Suppress("ForbiddenComment") class TestConfig internal constructor( - // command line execution command, use shell like "cmd", "bash" or other - val executionCommand: String, - // expected result file can be a full path or a relative path to a resource - val expectedResultFile: String, - // testFile can be a full path or a relative path to a resource - val testFile: String, - // executionType that controls processing of the test (like COMPARE, MIXED, CHECK_WARN, e.t.c) - val executionType: ExecutionType, - @SerialName("profile") val testProfile: TestProfile, - // option that controls if changes and automatic fix should be done directly in file - val inPlace: Boolean = false + val executionCommand: String, + val expectedResultFile: String, + val testFile: String, + val executionType: ExecutionType, + @SerialName("profile") val testProfile: TestProfile, + val inPlace: Boolean = false ) { /** * test name - it is not included in config content, but is injected on runtime by setter diff --git a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt index 1175a5cec4..d4abc3f55f 100644 --- a/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt +++ b/diktat-test-framework/src/main/kotlin/org/cqfn/diktat/test/framework/config/TestConfigReader.kt @@ -1,14 +1,15 @@ package org.cqfn.diktat.test.framework.config -import kotlinx.serialization.decodeFromString -import kotlinx.serialization.json.Json import java.io.BufferedReader import java.io.IOException import java.util.stream.Collectors +import kotlinx.serialization.decodeFromString +import kotlinx.serialization.json.Json import org.cqfn.diktat.common.config.reader.JsonResourceConfigReader /** * A [JsonResourceConfigReader] to read tests configuration as [TestConfig] + * @property classLoader a [ClassLoader] to load configutation file */ class TestConfigReader(configFilePath: String, override val classLoader: ClassLoader) : JsonResourceConfigReader() { /** From b86ff958f84ad3d32650d3ead563fb997e25ad83 Mon Sep 17 00:00:00 2001 From: Peter Trifanov Date: Thu, 15 Oct 2020 13:19:31 +0300 Subject: [PATCH 3/3] Making diktat consistent with own code style ### What's done: * Code style --- .../common/config/rules/RulesConfigReader.kt | 2 +- .../cqfn/diktat/ruleset/constants/Warnings.kt | 4 +- .../diktat/ruleset/generation/Generation.kt | 9 +- .../ruleset/rules/AnnotationNewLineRule.kt | 11 +- .../ruleset/rules/AvoidNestedFunctionsRule.kt | 17 +- .../ruleset/rules/BlockStructureBraces.kt | 26 ++- .../rules/BracesInConditionalsAndLoopsRule.kt | 32 +++- .../rules/ClassLikeStructuresOrderRule.kt | 70 +++++-- .../calculations/AccurateCalculationsRule.kt | 172 +++++++++--------- 9 files changed, 206 insertions(+), 137 deletions(-) diff --git a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt index e3462ec60e..358f989687 100644 --- a/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt +++ b/diktat-common/src/main/kotlin/org/cqfn/diktat/common/config/rules/RulesConfigReader.kt @@ -59,7 +59,7 @@ open class RulesConfigReader(override val classLoader: ClassLoader) : JsonResour * @param fileStream a [BufferedReader] representing loaded rules config file * @return list of [RulesConfig] */ - override fun parseResource(fileStream: BufferedReader) = fileStream.use { stream -> + override fun parseResource(fileStream: BufferedReader): List = fileStream.use { stream -> yamlSerializer.decodeFromString(stream.readLines().joinToString(separator = "\n")) } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt index aaf22756e7..726fc18709 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/constants/Warnings.kt @@ -10,7 +10,7 @@ import org.jetbrains.kotlin.com.intellij.lang.ASTNode * This class represent individual inspections of diktat code style. * A [Warnings] entry contains rule name, warning message and is used in code check. */ -@Suppress("ForbiddenComment", "MagicNumber") +@Suppress("ForbiddenComment", "MagicNumber", "WRONG_DECLARATIONS_ORDER") enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: String) : Rule { // ======== chapter 1 ======== PACKAGE_NAME_MISSING(true, "no package name declared in a file"), @@ -107,7 +107,6 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S // FixMe: change float literal to BigDecimal? Or kotlin equivalent? FLOAT_IN_ACCURATE_CALCULATIONS(false, "floating-point values shouldn't be used in accurate calculations"), - // ======== chapter 5 ======== TOO_LONG_FUNCTION(false, "function is too long: split it or make more primitive"), AVOID_NESTED_FUNCTIONS(true, "try to avoid using nested functions"), @@ -147,7 +146,6 @@ enum class Warnings(private val canBeAutoCorrected: Boolean, private val warn: S freeText: String, offset: Int, node: ASTNode) { - if (configs.isRuleEnabled(this) && !node.hasSuppress(name)) { val trimmedFreeText = freeText .lines() diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/generation/Generation.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/generation/Generation.kt index 981035b1b1..aec8a2b536 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/generation/Generation.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/generation/Generation.kt @@ -10,7 +10,7 @@ import org.cqfn.diktat.ruleset.rules.comments.HeaderCommentRule.Companion.afterC import org.cqfn.diktat.ruleset.rules.comments.HeaderCommentRule.Companion.curYear import org.cqfn.diktat.ruleset.rules.comments.HeaderCommentRule.Companion.hyphenRegex -private val AUTO_GENERATION_COMMENT = +private val autoGenerationComment = """ | This document was auto generated, please don't modify it. | This document contains all enum properties from Warnings.kt as Strings. @@ -25,7 +25,8 @@ private fun generateWarningNames() { val enumValNames = Warnings.values().map { it.name } val propertyList = enumValNames.map { - PropertySpec.builder(it, String::class) + PropertySpec + .builder(it, String::class) .addModifiers(KModifier.CONST) .initializer("\"$it\"") .build() @@ -40,10 +41,10 @@ private fun generateWarningNames() { .builder("generated", "WarningNames") .addType(fileBody) .indent(" ") - .addComment(AUTO_GENERATION_COMMENT) + .addComment(autoGenerationComment) .build() - kotlinFile.writeTo(File("diktat-rules/src/main/kotlin")) // fixme: need to add it to pom + kotlinFile.writeTo(File("diktat-rules/src/main/kotlin")) // fixme: need to add it to pom } private fun validateYear() { diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AnnotationNewLineRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AnnotationNewLineRule.kt index 1473241580..4964e3818e 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AnnotationNewLineRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AnnotationNewLineRule.kt @@ -23,9 +23,8 @@ import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl * This rule makes each annotation applied to a class, method or constructor is on its own line. Except: if first annotation of constructor, class or method */ class AnnotationNewLineRule(private val configRules: List) : Rule("annotation-new-line") { - - private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) private var isFixMode: Boolean = false + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) override fun visit(node: ASTNode, autoCorrect: Boolean, @@ -38,7 +37,6 @@ class AnnotationNewLineRule(private val configRules: List) : Rule(" } } - private fun checkAnnotation(node: ASTNode) { node.findChildByType(MODIFIER_LIST)?.let { modList -> fixAnnotation(modList) @@ -46,14 +44,15 @@ class AnnotationNewLineRule(private val configRules: List) : Rule(" } private fun fixAnnotation(node: ASTNode) { - if (node.getAllChildrenWithType(ANNOTATION_ENTRY).size <= 1) + if (node.getAllChildrenWithType(ANNOTATION_ENTRY).size <= 1) { return + } node.getAllChildrenWithType(ANNOTATION_ENTRY).forEach { - if (!it.isFollowedByNewline() || !it.isBeginByNewline()) + if (!it.isFollowedByNewline() || !it.isBeginByNewline()) { deleteSpaces(it, !it.isFollowedByNewline(), !it.isBeginByNewline()) + } } - } private fun deleteSpaces(node: ASTNode, rightSide: Boolean, leftSide: Boolean) { diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AvoidNestedFunctionsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AvoidNestedFunctionsRule.kt index 4cb121559f..8f375031a7 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AvoidNestedFunctionsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/AvoidNestedFunctionsRule.kt @@ -24,8 +24,8 @@ import org.jetbrains.kotlin.psi.psiUtil.parents * This rule checks for nested functions and warns if it finds any. */ class AvoidNestedFunctionsRule(private val configRules: List) : Rule("avoid-nested-functions") { - private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) private var isFixMode: Boolean = false + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) override fun visit(node: ASTNode, autoCorrect: Boolean, emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { emitWarn = emit @@ -46,7 +46,10 @@ class AvoidNestedFunctionsRule(private val configRules: List) : Rul canBeAutoCorrected = checkFunctionReferences(node)) { // We take last nested function, then add and remove child from bottom to top val lastFunc = node.findAllNodesWithSpecificType(FUN).last() - val funcSeq = lastFunc.parents().filter { it.elementType == FUN }.toMutableList() + val funcSeq = lastFunc + .parents() + .filter { it.elementType == FUN } + .toMutableList() funcSeq.add(0, lastFunc) val firstFunc = funcSeq.last() funcSeq.dropLast(1).forEachIndexed { index, it -> @@ -65,9 +68,11 @@ class AvoidNestedFunctionsRule(private val configRules: List) : Rul private fun isNestedFunction(node: ASTNode): Boolean = node.hasParent(FUN) && node.hasFunParentUntil(CLASS_BODY) && !node.hasChildOfType(MODIFIER_LIST) - private fun ASTNode.hasFunParentUntil(stopNode: IElementType): Boolean = - parents().asSequence().takeWhile { it.elementType != stopNode }.any { it.elementType == FUN } + parents() + .asSequence() + .takeWhile { it.elementType != stopNode } + .any { it.elementType == FUN } /** * Checks if local function has no usage of outside properties @@ -85,10 +90,10 @@ class AvoidNestedFunctionsRule(private val configRules: List) : Rul /** * Collects all function parameters' names + * * @return List of names */ @Suppress("UnsafeCallOnNullableType") private fun getParameterNames(node: ASTNode): List = - (node.psi as KtFunction).valueParameters.map { it.name!! } - + (node.psi as KtFunction).valueParameters.map { it.name!! } } diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BlockStructureBraces.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BlockStructureBraces.kt index 0c19af22f7..80887bb4dd 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BlockStructureBraces.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BlockStructureBraces.kt @@ -29,6 +29,7 @@ import com.pinterest.ktlint.core.ast.ElementType.WHILE import com.pinterest.ktlint.core.ast.ElementType.WHILE_KEYWORD import com.pinterest.ktlint.core.ast.ElementType.WHITE_SPACE import com.pinterest.ktlint.core.ast.isWhiteSpaceWithNewline +import java.lang.Exception import org.cqfn.diktat.common.config.rules.RuleConfiguration import org.cqfn.diktat.common.config.rules.RulesConfig import org.cqfn.diktat.common.config.rules.getRuleConfig @@ -44,12 +45,10 @@ import org.jetbrains.kotlin.com.intellij.psi.impl.source.tree.PsiWhiteSpaceImpl import org.jetbrains.kotlin.com.intellij.psi.tree.IElementType import org.jetbrains.kotlin.psi.KtIfExpression import org.jetbrains.kotlin.psi.KtTryExpression -import java.lang.Exception class BlockStructureBraces(private val configRules: List) : Rule("block-structure") { - - private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) private var isFixMode: Boolean = false + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) override fun visit(node: ASTNode, autoCorrect: Boolean, @@ -74,8 +73,9 @@ class BlockStructureBraces(private val configRules: List) : Rule("b private fun checkLambda(node: ASTNode, configuration: BlockStructureBracesConfiguration) { val isSingleLineLambda = node.text.lines().size == 1 - if (!isSingleLineLambda) + if (!isSingleLineLambda) { checkCloseBrace(node, configuration) + } } @Suppress("UnsafeCallOnNullableType") @@ -123,8 +123,11 @@ class BlockStructureBraces(private val configRules: List) : Rule("b private fun checkWhen(node: ASTNode, configuration: BlockStructureBracesConfiguration) { /// WHEN expression doesn't contain BLOCK element and LBRECE isn't the first child, so we should to find it. - val childrenAfterLBrace = node.getChildren(null).toList().run { subList(indexOfFirst { it.elementType == LBRACE }, size) } - if (!emptyBlockList.containsAll(childrenAfterLBrace.distinct().map { it.elementType })) { + val childrenAfterLbrace = node + .getChildren(null) + .toList() + .run { subList(indexOfFirst { it.elementType == LBRACE }, size) } + if (!emptyBlockList.containsAll(childrenAfterLbrace.distinct().map { it.elementType })) { checkOpenBraceOnSameLine(node, LBRACE, configuration) checkCloseBrace(node, configuration) } @@ -160,7 +163,9 @@ class BlockStructureBraces(private val configRules: List) : Rule("b } private fun checkOpenBraceOnSameLine(node: ASTNode, beforeType: IElementType, configuration: BlockStructureBracesConfiguration) { - if (!configuration.openBrace) return + if (!configuration.openBrace) { + return + } val nodeBefore = node.findChildByType(beforeType) val braceSpace = nodeBefore?.treePrev if (braceSpace == null || checkBraceNode(braceSpace, true)) { @@ -210,9 +215,11 @@ class BlockStructureBraces(private val configRules: List) : Rule("b @Suppress("UnsafeCallOnNullableType") private fun checkCloseBrace(node: ASTNode, configuration: BlockStructureBracesConfiguration) { - if (!configuration.closeBrace) return + if (!configuration.closeBrace) { + return + } val space = node.findChildByType(RBRACE)!!.treePrev - if (checkBraceNode(space)) + if (checkBraceNode(space)) { BRACES_BLOCK_STRUCTURE_ERROR.warnAndFix(configRules, emitWarn, isFixMode, "no newline before closing brace", (space.treeNext ?: node.findChildByType(RBRACE))!!.startOffset, node) { if (space.elementType != WHITE_SPACE) { @@ -221,6 +228,7 @@ class BlockStructureBraces(private val configRules: List) : Rule("b (space as LeafPsiElement).replaceWithText("\n") } } + } } private fun checkBraceNode(node: ASTNode, shouldContainNewline: Boolean = false) = diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BracesInConditionalsAndLoopsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BracesInConditionalsAndLoopsRule.kt index 08b1abcdce..26251c15a2 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BracesInConditionalsAndLoopsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/BracesInConditionalsAndLoopsRule.kt @@ -33,12 +33,8 @@ import org.jetbrains.kotlin.psi.KtWhenExpression import org.jetbrains.kotlin.psi.psiUtil.astReplace class BracesInConditionalsAndLoopsRule(private val configRules: List) : Rule("braces-rule") { - companion object { - private const val indentStep = 4 - } - - private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) private var isFixMode: Boolean = false + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) override fun visit(node: ASTNode, autoCorrect: Boolean, @@ -62,9 +58,16 @@ class BracesInConditionalsAndLoopsRule(private val configRules: List) : private var isFixMode: Boolean = false private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) + /** + * @param node + * @param autoCorrect + * @param emit + */ override fun visit(node: ASTNode, autoCorrect: Boolean, emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { @@ -72,29 +77,42 @@ class ClassLikeStructuresOrderRule(private val configRules: List) : val initBlocks = node.getChildren(TokenSet.create(CLASS_INITIALIZER)).toMutableList() val constructors = node.getChildren(TokenSet.create(SECONDARY_CONSTRUCTOR)).toMutableList() val methods = node.getChildren(TokenSet.create(FUN)).toMutableList() - val (usedClasses, unusedClasses) = node.getChildren(TokenSet.create(CLASS)).partition { classNode -> - classNode.getIdentifierName()?.let { identifierNode -> - node.parents().last().findAllNodesWithSpecificType(REFERENCE_EXPRESSION).any { ref -> - ref.parent({ it == classNode }) == null && ref.text.contains(identifierNode.text) + val (usedClasses, unusedClasses) = node + .getChildren(TokenSet.create(CLASS)) + .partition { classNode -> + classNode.getIdentifierName()?.let { identifierNode -> + node + .parents() + .last() + .findAllNodesWithSpecificType(REFERENCE_EXPRESSION) + .any { ref -> + ref.parent({ it == classNode }) == null && ref.text.contains(identifierNode.text) + } + } ?: false } - } ?: false - }.let { it.first.toMutableList() to it.second.toMutableList() } + .let { it.first.toMutableList() to it.second.toMutableList() } val companion = node.getChildren(TokenSet.create(OBJECT_DECLARATION)) .find { it.findChildByType(MODIFIER_LIST)?.findLeafWithSpecificType(COMPANION_KEYWORD) != null } val blocks = Blocks(AllProperties(loggers, constProperties, properties, lateInitProperties), initBlocks, constructors, methods, usedClasses, listOfNotNull(companion).toMutableList(), unusedClasses) - blocks.allBlockFlattened().reversed().handleIncorrectOrder(blocks::getSiblingBlocks) { astNode, beforeThisNode -> - WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES.warnAndFix(configRules, emitWarn, isFixMode, astNode.elementType.toString() + ": " + astNode.text, astNode.startOffset, astNode) { - val replacement = node.moveChildBefore(astNode, beforeThisNode, true) - replacement.oldNodes.forEachIndexed { idx, oldNode -> - blocks.allBlocks().find { oldNode in it }?.apply { - this[indexOf(oldNode)] = replacement.newNodes[idx] + blocks + .allBlockFlattened() + .reversed() + .handleIncorrectOrder(blocks::getSiblingBlocks) { astNode, beforeThisNode -> + WRONG_ORDER_IN_CLASS_LIKE_STRUCTURES.warnAndFix(configRules, emitWarn, isFixMode, "${astNode.elementType}: ${astNode.text}", astNode.startOffset, astNode) { + val replacement = node.moveChildBefore(astNode, beforeThisNode, true) + replacement.oldNodes.forEachIndexed { idx, oldNode -> + blocks + .allBlocks() + .find { oldNode in it } + ?.apply { + this[indexOf(oldNode)] = replacement.newNodes[idx] + } + } } } - } - } } @Suppress("UnsafeCallOnNullableType") @@ -132,11 +150,28 @@ class ClassLikeStructuresOrderRule(private val configRules: List) : } } + /** + * Data class containing different groups of properties in file + * + * @property loggers loggers (for example, properties called `log` or `logger`) + * @property constProperties `const val`s + * @property properties all other properties + * @property lateInitProperties `lateinit var`s + */ private data class AllProperties(val loggers: MutableList, val constProperties: MutableList, val properties: MutableList, val lateInitProperties: MutableList) + /** + * @property allProperties an instance of [AllProperties] + * @property initBlocks `init` blocks + * @property constructors constructors + * @property methods functions + * @property usedClasses nested classes that are used in the enclosing class + * @property companion `companion object`s + * @property unusedClasses nested classes that are *not* used in the enclosing class + */ private data class Blocks(val allProperties: AllProperties, val initBlocks: MutableList, val constructors: MutableList, @@ -148,6 +183,9 @@ class ClassLikeStructuresOrderRule(private val configRules: List) : require(companion.size in 0..1) } + /** + * @return all groups of structures in the class + */ fun allBlocks() = with(allProperties) { listOf(loggers, constProperties, properties, lateInitProperties, initBlocks, constructors, methods, usedClasses, companion, unusedClasses) @@ -155,6 +193,10 @@ class ClassLikeStructuresOrderRule(private val configRules: List) : fun allBlockFlattened() = allBlocks().flatten() + /** + * @param node + * @return nodes between which the [node] should be placed + */ fun getSiblingBlocks(node: ASTNode): Pair { require(node in allBlockFlattened()) val lastElement = allBlockFlattened().last() diff --git a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt index d810f293e0..2f3bb876b1 100644 --- a/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt +++ b/diktat-rules/src/main/kotlin/org/cqfn/diktat/ruleset/rules/calculations/AccurateCalculationsRule.kt @@ -24,47 +24,63 @@ import org.jetbrains.kotlin.psi.psiUtil.startOffset * Fixme: detect variables by type, not only floating-point literals */ class AccurateCalculationsRule(private val configRules: List) : Rule("accurate-calculations") { - private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) private var isFixMode: Boolean = false + private lateinit var emitWarn: ((offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) - companion object { - private val arithmeticOperationTokens = listOf(KtTokens.PLUS, KtTokens.PLUSEQ, KtTokens.PLUSPLUS, - KtTokens.MINUS, KtTokens.MINUSEQ, KtTokens.MINUSMINUS, - KtTokens.MUL, KtTokens.MULTEQ, KtTokens.DIV, KtTokens.DIVEQ, - KtTokens.PERC, KtTokens.PERCEQ, - KtTokens.GT, KtTokens.LT, KtTokens.LTEQ, KtTokens.GTEQ, - KtTokens.EQEQ - ) - private val comparisonOperators = listOf(KtTokens.LT, KtTokens.LTEQ, KtTokens.GT, KtTokens.GTEQ) - private val arithmeticOperationsFunctions = listOf("equals", "compareTo") - private val comparisonFunctions = listOf("compareTo") - } + private fun KtCallExpression?.isAbsOfFloat() = this + ?.run { + (calleeExpression as? KtNameReferenceExpression) + ?.getReferencedName() + ?.equals("abs") + ?.and( + valueArguments + .singleOrNull() + ?.getArgumentExpression() + ?.isFloatingPoint() + ?: false) + ?: false + } + ?: false + private fun KtDotQualifiedExpression.isComparisonWithAbs() = + takeIf { + it.selectorExpression.run { + this is KtCallExpression && getFunctionName() in comparisonFunctions + } + } + ?.run { + (selectorExpression as KtCallExpression) + .valueArguments + .singleOrNull() + ?.let { it.getArgumentExpression() as? KtCallExpression } + ?.isAbsOfFloat() + ?: false || + (receiverExpression as? KtCallExpression).isAbsOfFloat() + } + ?: false - override fun visit(node: ASTNode, - autoCorrect: Boolean, - emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { - emitWarn = emit - isFixMode = autoCorrect + private fun KtBinaryExpression.isComparisonWithAbs() = + takeIf { it.operationToken in comparisonOperators } + ?.run { left as? KtCallExpression ?: right as? KtCallExpression } + ?.run { calleeExpression as? KtNameReferenceExpression } + ?.getReferencedName() + ?.equals("abs") + ?: false - when (val psi = node.psi) { - is KtBinaryExpression -> handleBinaryExpression(psi) - is KtDotQualifiedExpression -> handleFunction(psi) - else -> return + private fun isComparisonWithAbs(psiElement: PsiElement) = + when (psiElement) { + is KtBinaryExpression -> psiElement.isComparisonWithAbs() + is KtDotQualifiedExpression -> psiElement.isComparisonWithAbs() + else -> false + } + + private fun checkFloatValue(floatValue: PsiElement?, expression: KtExpression) { + if (floatValue != null) { + // float value is used in comparison + FLOAT_IN_ACCURATE_CALCULATIONS.warn(configRules, emitWarn, isFixMode, + "float value of <${floatValue.text}> used in arithmetic expression in ${expression.text}", expression.startOffset, expression.node) } } - @Suppress("UnsafeCallOnNullableType") - private fun handleBinaryExpression(expression: KtBinaryExpression) = expression - .takeIf { it.operationToken in arithmeticOperationTokens } - ?.takeUnless { it.parentsWithSelf.any(::isComparisonWithAbs) } - ?.run { - // !! is safe because `KtBinaryExpression#left` is annotated `Nullable IfNotParsed` - val floatValue = left!!.takeIf { it.isFloatingPoint() } - ?: right!!.takeIf { it.isFloatingPoint() } - checkFloatValue(floatValue, this) - println() - } - private fun handleFunction(expression: KtDotQualifiedExpression) = expression .takeIf { it.selectorExpression is KtCallExpression } ?.run { receiverExpression to selectorExpression as KtCallExpression } @@ -79,60 +95,48 @@ class AccurateCalculationsRule(private val configRules: List) : Rul checkFloatValue(floatValue, expression) } - private fun checkFloatValue(floatValue: PsiElement?, expression: KtExpression) { - if (floatValue != null) { - // float value is used in comparison - FLOAT_IN_ACCURATE_CALCULATIONS.warn(configRules, emitWarn, isFixMode, - "float value of <${floatValue.text}> used in arithmetic expression in ${expression.text}", expression.startOffset, expression.node) - } - } - - private fun isComparisonWithAbs(psiElement: PsiElement) = - when (psiElement) { - is KtBinaryExpression -> psiElement.isComparisonWithAbs() - is KtDotQualifiedExpression -> psiElement.isComparisonWithAbs() - else -> false + @Suppress("UnsafeCallOnNullableType") + private fun handleBinaryExpression(expression: KtBinaryExpression) = expression + .takeIf { it.operationToken in arithmeticOperationTokens } + ?.takeUnless { it.parentsWithSelf.any(::isComparisonWithAbs) } + ?.run { + // !! is safe because `KtBinaryExpression#left` is annotated `Nullable IfNotParsed` + val floatValue = left!!.takeIf { it.isFloatingPoint() } + ?: right!!.takeIf { it.isFloatingPoint() } + checkFloatValue(floatValue, this) + println() } - private fun KtBinaryExpression.isComparisonWithAbs() = - takeIf { it.operationToken in comparisonOperators } - ?.run { left as? KtCallExpression ?: right as? KtCallExpression } - ?.run { calleeExpression as? KtNameReferenceExpression } - ?.getReferencedName() - ?.equals("abs") - ?: false + /** + * @param node + * @param autoCorrect + * @param emit + */ + override fun visit(node: ASTNode, + autoCorrect: Boolean, + emit: (offset: Int, errorMessage: String, canBeAutoCorrected: Boolean) -> Unit) { + emitWarn = emit + isFixMode = autoCorrect - private fun KtDotQualifiedExpression.isComparisonWithAbs() = - takeIf { - it.selectorExpression.run { - this is KtCallExpression && getFunctionName() in comparisonFunctions - } - } - ?.run { - (selectorExpression as KtCallExpression) - .valueArguments - .singleOrNull() - ?.let { it.getArgumentExpression() as? KtCallExpression } - ?.isAbsOfFloat() - ?: false || - (receiverExpression as? KtCallExpression).isAbsOfFloat() - } - ?: false + when (val psi = node.psi) { + is KtBinaryExpression -> handleBinaryExpression(psi) + is KtDotQualifiedExpression -> handleFunction(psi) + else -> return + } + } - private fun KtCallExpression?.isAbsOfFloat() = this - ?.run { - (calleeExpression as? KtNameReferenceExpression) - ?.getReferencedName() - ?.equals("abs") - ?.and( - valueArguments - .singleOrNull() - ?.getArgumentExpression() - ?.isFloatingPoint() - ?: false) - ?: false - } - ?: false + companion object { + private val arithmeticOperationTokens = listOf(KtTokens.PLUS, KtTokens.PLUSEQ, KtTokens.PLUSPLUS, + KtTokens.MINUS, KtTokens.MINUSEQ, KtTokens.MINUSMINUS, + KtTokens.MUL, KtTokens.MULTEQ, KtTokens.DIV, KtTokens.DIVEQ, + KtTokens.PERC, KtTokens.PERCEQ, + KtTokens.GT, KtTokens.LT, KtTokens.LTEQ, KtTokens.GTEQ, + KtTokens.EQEQ + ) + private val comparisonOperators = listOf(KtTokens.LT, KtTokens.LTEQ, KtTokens.GT, KtTokens.GTEQ) + private val arithmeticOperationsFunctions = listOf("equals", "compareTo") + private val comparisonFunctions = listOf("compareTo") + } } @Suppress("UnsafeCallOnNullableType")