From b0c52ffc70e12c5781ed544fedc47a0cd3eb5fe2 Mon Sep 17 00:00:00 2001 From: Steffen Wonning Date: Sun, 14 Jan 2024 13:59:03 +0100 Subject: [PATCH] Reduce code complexity which generates the directives (#79) * Improve documentation * Update basic directive tests * Reduce complexity from the directive write process * Update library keyword usage --- .../dartpoet/directive/BaseDirective.kt | 5 +- .../dartpoet/directive/DartDirective.kt | 55 ++++++------- .../dartpoet/directive/ExportDirective.kt | 28 +++---- .../dartpoet/directive/LibraryDirective.kt | 15 ++-- .../dartpoet/directive/RelativeDirective.kt | 13 +++- .../dartpoet/directive/DirectiveTest.kt | 78 +++++++++++-------- 6 files changed, 105 insertions(+), 89 deletions(-) diff --git a/src/main/kotlin/net/theevilreaper/dartpoet/directive/BaseDirective.kt b/src/main/kotlin/net/theevilreaper/dartpoet/directive/BaseDirective.kt index 88c253e1..dd372d82 100644 --- a/src/main/kotlin/net/theevilreaper/dartpoet/directive/BaseDirective.kt +++ b/src/main/kotlin/net/theevilreaper/dartpoet/directive/BaseDirective.kt @@ -5,13 +5,14 @@ import net.theevilreaper.dartpoet.code.buildCodeString import net.theevilreaper.dartpoet.util.DART_FILE_ENDING /** - * Represents the base implementation for each [Directive]. + * The class represents the basic implementation which are used from all directive implementations. + * @param path the path for the directive * @author theEvilReaper * @since 1.0.0 */ abstract class BaseDirective( private val path: String -): Directive { +) : Directive { init { check(path.trim().isNotEmpty()) { "The path of an directive can't be empty" } diff --git a/src/main/kotlin/net/theevilreaper/dartpoet/directive/DartDirective.kt b/src/main/kotlin/net/theevilreaper/dartpoet/directive/DartDirective.kt index d8c50aee..b7a898f9 100644 --- a/src/main/kotlin/net/theevilreaper/dartpoet/directive/DartDirective.kt +++ b/src/main/kotlin/net/theevilreaper/dartpoet/directive/DartDirective.kt @@ -5,17 +5,15 @@ import net.theevilreaper.dartpoet.util.IMPORT import net.theevilreaper.dartpoet.util.SEMICOLON /** - * The class represents a normal import from dart. - * An import statement can look like this in dart: - *
    - *
  1. 'package:flutter/material.dart'
    2. - *
  2. '../../model/item_model.dart'
  3. - *
- * Dart also allows to add a prefix to an import which means that an import can look like that: - * -> import ../../model/item_model.dart as itemModel; + * Represents an import directive from dart which usual starts with `dart` or `package`. * - * @author theEvilReaper - * @since 1.0.0 + * @param path the path to the Dart file or package being imported. + * @param castType the optional cast type for the imported directive, used when casting the directive + * @param importCast the optional import cast, specifying a cast expression for the imported directive. + * + * @throws IllegalArgumentException if [importCast] is provided and is empty or consists only of whitespace. + * + * @constructor Creates a Dart import directive with the specified path as [String], a cast type as [CastType], and a importCast a [String]. */ class DartDirective( private val path: String, @@ -27,32 +25,29 @@ class DartDirective( * Check if some conditions are false and throw an exception. */ init { - if (castType != null && importCast != null) { + if (importCast != null) { check(importCast.trim().isNotEmpty()) { "The importCast can't be empty" } } + + if ((castType != null && importCast == null) || (castType == null && importCast != null)) { + throw IllegalStateException("The castType and importCast must be set together or must be null. A mixed state is not allowed") + } } + /** + * Writes the given data from the directive to the provided [CodeWriter]. + * + * @param writer the writer instance to append the directive + */ override fun write(writer: CodeWriter) { writer.emit("$IMPORT ") - if (importCast == null && castType == null) { - if (isDartImport()) { - writer.emit("'${path.ensureDartFileEnding()}'") - } else { - writer.emit("'") - writer.emit("package:") - writer.emit(path.ensureDartFileEnding()) - writer.emit("'") - } - writer.emit(SEMICOLON) - } else if (importCast != null && castType != null) { - writer.emit("'") - if (!isDartImport()) { - writer.emit("package:") - } - writer.emit("${path.ensureDartFileEnding()}' ${castType.identifier} $importCast") - writer.emit(SEMICOLON) - } else { - throw Error("Something went wrong during the DartDirective write prozess") + val ensuredPath = path.ensureDartFileEnding() + val pathToWrite = if (isDartImport()) ensuredPath else "package:$ensuredPath" + writer.emit("'$pathToWrite'") + + if (importCast != null && castType != null) { + writer.emit(" ${castType.identifier} $importCast") } + writer.emit(SEMICOLON) } } diff --git a/src/main/kotlin/net/theevilreaper/dartpoet/directive/ExportDirective.kt b/src/main/kotlin/net/theevilreaper/dartpoet/directive/ExportDirective.kt index b48965bb..405736d3 100644 --- a/src/main/kotlin/net/theevilreaper/dartpoet/directive/ExportDirective.kt +++ b/src/main/kotlin/net/theevilreaper/dartpoet/directive/ExportDirective.kt @@ -16,21 +16,23 @@ class ExportDirective( private val path: String, private val castType: CastType? = null, private val importCast: String? = null, -): BaseDirective(path) { +) : BaseDirective(path) { private val export = "export" private val invalidCastType = arrayOf(CastType.DEFERRED, CastType.AS) init { - if (castType != null) { - if (castType in invalidCastType) { - throw IllegalStateException("The following cast types are not allowed for an export directive: [${invalidCastType.joinToString()}]") - } + if (castType != null && castType in invalidCastType) { + throw IllegalStateException("The following cast types are not allowed for an export directive: [${invalidCastType.joinToString()}]") } if (importCast != null) { check(importCast.trim().isNotEmpty()) { "The importCast can't be empty" } } + + if ((castType != null && importCast == null) || (castType == null && importCast != null)) { + throw IllegalStateException("The castType and importCast must be set together or must be null. A mixed state is not allowed") + } } /** @@ -38,16 +40,14 @@ class ExportDirective( * @param writer the [CodeWriter] instance to append the directive */ override fun write(writer: CodeWriter) { + val ensuredPath = path.ensureDartFileEnding() writer.emit("$export·'") - if (importCast == null && castType == null) { - writer.emit(path.ensureDartFileEnding()) - writer.emit("'$SEMICOLON") - } else if (importCast != null && castType != null) { - writer.emit(path.ensureDartFileEnding()) - writer.emit("' ${castType.identifier} $importCast") - writer.emit(SEMICOLON) - } else { - throw Error("Something went wrong during the ExportDirective write process") + writer.emit(ensuredPath) + writer.emit("'") + + if (importCast != null && castType != null) { + writer.emit("·${castType.identifier} $importCast") } + writer.emit(SEMICOLON) } } diff --git a/src/main/kotlin/net/theevilreaper/dartpoet/directive/LibraryDirective.kt b/src/main/kotlin/net/theevilreaper/dartpoet/directive/LibraryDirective.kt index b0fca6c6..575427e7 100644 --- a/src/main/kotlin/net/theevilreaper/dartpoet/directive/LibraryDirective.kt +++ b/src/main/kotlin/net/theevilreaper/dartpoet/directive/LibraryDirective.kt @@ -5,21 +5,22 @@ import net.theevilreaper.dartpoet.code.CodeWriter import net.theevilreaper.dartpoet.util.SEMICOLON /** - * + * The [LibraryDirective] represents the library directive from dart. * @since 1.0.0 * @author theEvilReaper */ class LibraryDirective( private val path: String, private val asPartOf: Boolean = false -): BaseDirective(path) { +) : BaseDirective(path) { + /** + * Writes the data from the [LibraryDirective] to a given instance from a [CodeWriter]. + * @param writer the [CodeWriter] instance to append the directive + */ override fun write(writer: CodeWriter) { - if (asPartOf) { - writer.emit("part of ") - } else { - writer.emit("${LIBRARY.identifier} ") - } + val baseString = if (asPartOf) "part of" else LIBRARY.identifier + writer.emit("$baseString·") writer.emit(path) writer.emit(SEMICOLON) } diff --git a/src/main/kotlin/net/theevilreaper/dartpoet/directive/RelativeDirective.kt b/src/main/kotlin/net/theevilreaper/dartpoet/directive/RelativeDirective.kt index 2b60ecc6..10a7fc60 100644 --- a/src/main/kotlin/net/theevilreaper/dartpoet/directive/RelativeDirective.kt +++ b/src/main/kotlin/net/theevilreaper/dartpoet/directive/RelativeDirective.kt @@ -13,7 +13,17 @@ class RelativeDirective( private val path: String, private val castType: CastType? = null, private val importCast: String? = null, -): BaseDirective(path) { +) : BaseDirective(path) { + + init { + if (importCast != null) { + check(importCast.trim().isNotEmpty()) { "The importCast can't be empty" } + } + + if ((castType != null && importCast == null) || (castType == null && importCast != null)) { + throw IllegalStateException("The castType and importCast must be set together or must be null. A mixed state is not allowed") + } + } /** * Writes the content for a relative directive to an instance of an [CodeWriter]. @@ -28,7 +38,6 @@ class RelativeDirective( if (castType != null && importCast != null) { writer.emit(" ${castType.identifier} $importCast") } - writer.emit(SEMICOLON) } } diff --git a/src/test/kotlin/net/theevilreaper/dartpoet/directive/DirectiveTest.kt b/src/test/kotlin/net/theevilreaper/dartpoet/directive/DirectiveTest.kt index 40d60db2..9b184373 100644 --- a/src/test/kotlin/net/theevilreaper/dartpoet/directive/DirectiveTest.kt +++ b/src/test/kotlin/net/theevilreaper/dartpoet/directive/DirectiveTest.kt @@ -1,7 +1,8 @@ package net.theevilreaper.dartpoet.directive -import org.junit.jupiter.api.Assertions.* +import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Test +import org.junit.jupiter.api.assertThrows import org.junit.jupiter.params.ParameterizedTest import org.junit.jupiter.params.provider.Arguments import org.junit.jupiter.params.provider.MethodSource @@ -14,6 +15,24 @@ class DirectiveTest { companion object { private const val CAST_VALUE = "item" private const val TEST_IMPORT = "../../model/item_model.dart" + private const val EMPTY_NAME_MESSAGE = "The path of an directive can't be empty" + private const val INVALID_CAST_USAGE = + "The castType and importCast must be set together or must be null. A mixed state is not allowed" + + @JvmStatic + private fun directivesWhichThrowsException() = Stream.of( + Arguments.of({ DartDirective(" ") }, EMPTY_NAME_MESSAGE), + Arguments.of({ LibraryDirective("") }, EMPTY_NAME_MESSAGE), + Arguments.of({ PartDirective("") }, EMPTY_NAME_MESSAGE), + Arguments.of({ RelativeDirective("") }, EMPTY_NAME_MESSAGE), + Arguments.of({ LibraryDirective("") }, EMPTY_NAME_MESSAGE), + Arguments.of({ DartDirective("flutter/material.dart", CastType.AS, null) }, INVALID_CAST_USAGE), + Arguments.of({ DartDirective("flutter/material.dart", null, "test") }, INVALID_CAST_USAGE), + Arguments.of({ RelativeDirective("flutter/material.dart", CastType.AS, null) }, INVALID_CAST_USAGE), + Arguments.of({ RelativeDirective("flutter/material.dart", null, "test") }, INVALID_CAST_USAGE), + Arguments.of({ ExportDirective("test.dart", CastType.HIDE, null) }, INVALID_CAST_USAGE), + Arguments.of({ ExportDirective("test.dart", null, "test") }, INVALID_CAST_USAGE) + ) @JvmStatic private fun libDirectives() = Stream.of( @@ -30,10 +49,22 @@ class DirectiveTest { @JvmStatic private fun relativeDirectives() = Stream.of( Arguments.of(RelativeDirective(TEST_IMPORT), "import '../../model/item_model.dart';"), - Arguments.of(RelativeDirective(TEST_IMPORT, CastType.AS, CAST_VALUE), "import '../../model/item_model.dart' as item;"), - Arguments.of(RelativeDirective(TEST_IMPORT, CastType.DEFERRED, CAST_VALUE), "import '../../model/item_model.dart' deferred as item;"), - Arguments.of(RelativeDirective(TEST_IMPORT, CastType.HIDE, CAST_VALUE), "import '../../model/item_model.dart' hide item;"), - Arguments.of(RelativeDirective(TEST_IMPORT, CastType.SHOW, CAST_VALUE), "import '../../model/item_model.dart' show item;") + Arguments.of( + RelativeDirective(TEST_IMPORT, CastType.AS, CAST_VALUE), + "import '../../model/item_model.dart' as item;" + ), + Arguments.of( + RelativeDirective(TEST_IMPORT, CastType.DEFERRED, CAST_VALUE), + "import '../../model/item_model.dart' deferred as item;" + ), + Arguments.of( + RelativeDirective(TEST_IMPORT, CastType.HIDE, CAST_VALUE), + "import '../../model/item_model.dart' hide item;" + ), + Arguments.of( + RelativeDirective(TEST_IMPORT, CastType.SHOW, CAST_VALUE), + "import '../../model/item_model.dart' show item;" + ) ) @JvmStatic @@ -43,6 +74,14 @@ class DirectiveTest { ) } + @ParameterizedTest + @MethodSource("directivesWhichThrowsException") + fun `test directives which throws exception`(current: () -> Directive, expectedMessage: String) { + val exception = assertThrows { current() } + assertEquals(IllegalStateException::class.java, exception.javaClass) + assertEquals(expectedMessage, exception.message) + } + @ParameterizedTest @MethodSource("libDirectives") fun `test library imports`(current: Directive, expected: String) { @@ -67,35 +106,6 @@ class DirectiveTest { assertEquals(expected, current.asString()) } - @Test - fun `test import with empty path`() { - assertThrows( - IllegalStateException::class.java, - { DartDirective(" ") }, - "The path of an Import can't be empty" - ) - assertThrows( - IllegalStateException::class.java, - { DartDirective("") }, - "The path of an Import can't be empty" - ) - } - - @Test - fun `test cast import with empty cast`() { - assertThrows( - IllegalStateException::class.java, - { DartDirective("flutter/material.dart", CastType.AS, " ") }, - "The importCast can't be empty" - ) - assertThrows( - IllegalStateException::class.java, - { DartDirective("flutter/material.dart", CastType.AS, "") }, - "The importCast can't be empty" - ) - } - - @Test fun `test package import`() { val import = DartDirective("flutter/material.dart")