Skip to content

Commit 94de5a9

Browse files
authored
Remove Any from public APIs (#48)
* feat: add closed types for error context and sql parameters * refactor: type error context as ContextValue instead of Any * fix: remove unsound casts from core patterns * refactor: drop Any from contract and algebra signatures * ci: fail the build on Any or asInstanceOf in production code
1 parent e296c81 commit 94de5a9

23 files changed

Lines changed: 374 additions & 153 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ jobs:
6161
exit 1
6262
fi
6363
64+
- name: Guard - ban Any and asInstanceOf in production sources
65+
run: |
66+
chmod +x scripts/check-type-safety.sh
67+
./scripts/check-type-safety.sh
68+
6469
- name: compile all
6570
run: sbt clean compileAll
6671

‎.scalafix.conf‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@ DisableSyntax.noThrows = true
1313
DisableSyntax.noNulls = true
1414
DisableSyntax.noReturns = true
1515
DisableSyntax.noWhileLoops = true
16-
# asInstanceOf checked with whitelist in CI (scalafix doesn't support per-file excludes)
16+
# asInstanceOf and Any are banned by scripts/check-type-safety.sh, which runs in CI. They need a per-file
17+
# allowlist for the reflection boundaries, and DisableSyntax has no per-file excludes.
1718
DisableSyntax.noIsInstanceOf = true
1819
DisableSyntax.noXml = true
1920
DisableSyntax.regex = [
@@ -67,6 +68,5 @@ OrganizeImports.groupedImports = Merge
6768
# default Ascii order the two formatters each undo the other's selector order and neither check can pass.
6869
OrganizeImports.importSelectorsOrder = SymbolsFirst
6970

70-
# Note: Scalafix DisableSyntax doesn't support per-file excludes
71-
# Whitelists for asInstanceOf and scala.util.Try are enforced via shell scripts in CI
72-
# Examples directory is excluded from CI checks via grep filter (demo code, not production)
71+
# Note: Scalafix DisableSyntax doesn't support per-file excludes, so the checks that need an allowlist live
72+
# in scripts/check-type-safety.sh instead. Examples are demo code and excluded from both.

‎modules/connectors/src/main/scala/com/flowforge/connectors/safety/ConnectorErrorMapper.scala‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ package com.flowforge.connectors.safety
22

33
import com.flowforge.core.safety.ErrorMapper
44
import com.flowforge.core.types.FlowForgeError.{ ConfigurationError, ValidationError }
5-
import com.flowforge.core.types.SystemError
5+
import com.flowforge.core.types.{ ContextValue, SystemError }
66

77
/**
88
* Connector-focused ErrorMapper.
@@ -13,17 +13,25 @@ import com.flowforge.core.types.SystemError
1313
object ConnectorErrorMapper {
1414
implicit val connectorMapper: ErrorMapper = {
1515
case e: java.nio.file.NoSuchFileException =>
16-
ValidationError(s"File not found: ${e.getMessage}", None, context = Map("cause" -> "NoSuchFile"))
16+
ValidationError(
17+
s"File not found: ${e.getMessage}",
18+
None,
19+
context = Map("cause" -> ContextValue.Text("NoSuchFile")),
20+
)
1721
.withCause(e)
1822
case e: java.io.FileNotFoundException =>
19-
ValidationError(s"File not found: ${e.getMessage}", None, context = Map("cause" -> "FileNotFound"))
23+
ValidationError(
24+
s"File not found: ${e.getMessage}",
25+
None,
26+
context = Map("cause" -> ContextValue.Text("FileNotFound")),
27+
)
2028
.withCause(e)
2129
case e: java.io.IOException =>
2230
SystemError.ServiceUnavailable(serviceName = "filesystem", message = e.getMessage, cause = Some(e))
2331
case e: java.sql.SQLException =>
2432
ConfigurationError(
2533
s"JDBC error: ${e.getMessage}",
26-
context = Map("sqlState" -> String.valueOf(e.getSQLState)),
34+
context = Map("sqlState" -> ContextValue.Text(String.valueOf(e.getSQLState))),
2735
).withCause(e)
2836
case other => ErrorMapper.default(other)
2937
}

‎modules/contracts/src/main/scala/com/flowforge/contracts/DataContract.scala‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,11 @@ object FieldType {
7676
/** Algebraic data type for field‑level constraints. */
7777
sealed trait FieldConstraint
7878
object FieldConstraint {
79-
case class MinLength(value: Int) extends FieldConstraint
80-
case class MaxLength(value: Int) extends FieldConstraint
81-
case class Range(min: Double, max: Double) extends FieldConstraint
82-
case class Pattern(regex: Regex) extends FieldConstraint
83-
case class OneOf(values: Set[String]) extends FieldConstraint
84-
case class Custom(name: String, validator: Any => Boolean) extends FieldConstraint
79+
case class MinLength(value: Int) extends FieldConstraint
80+
case class MaxLength(value: Int) extends FieldConstraint
81+
case class Range(min: Double, max: Double) extends FieldConstraint
82+
case class Pattern(regex: Regex) extends FieldConstraint
83+
case class OneOf(values: Set[String]) extends FieldConstraint
8584
}
8685

8786
/** Composable validation rules evaluated at runtime. */
@@ -100,19 +99,21 @@ object RuleSeverity {
10099

101100
/** Common validation rules provided out of the box. */
102101
object ValidationRules {
103-
def nonNull[A](fieldName: String)(extract: A => Any): ValidationRule[A] =
102+
def nonNull[A](fieldName: String)(extract: A => AnyRef): ValidationRule[A] =
104103
new ValidationRule[A] {
105104
val name = s"nonNull($fieldName)"
106105
def validate(data: A): ValidatedNel[ContractViolation, Unit] =
107-
// `extract` returns Any because it reaches into caller data this module does not control, so a null
108-
// genuinely can arrive here. Option is the narrowest way to ask without naming the literal.
106+
// `extract` returns a reference because that is the only thing a null can be, and this rule exists to
107+
// catch a null arriving from caller data the module does not control. It was `A => Any`, which also
108+
// accepted a primitive the check could never fail for. `Option` is the narrowest way to ask without
109+
// naming the literal.
109110
Option(extract(data)) match {
110111
case Some(_) => ().validNel
111112
case None => ContractViolation.NullValue(fieldName).invalidNel
112113
}
113114
}
114115

115-
def unique[A](fieldName: String)(extract: A => Any): ValidationRule[A] =
116+
def unique[A, B](fieldName: String)(@annotation.unused extract: A => B): ValidationRule[A] =
116117
new ValidationRule[A] {
117118
val name = s"unique($fieldName)"
118119
def validate(data: A): ValidatedNel[ContractViolation, Unit] =

‎modules/core/src/main/scala/com/flowforge/core/algebra/DataAlgebra.scala‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -455,13 +455,18 @@ object DataAlgebra {
455455

456456
/**
457457
* Data profile for understanding dataset characteristics.
458+
*
459+
* `statistics` is numeric because a profile's statistics are: min, max, mean, stddev, percentiles. It was
460+
* `Map[String, Any]`, and every one of the three implementations passes `Map.empty`, so nothing was relying
461+
* on the wider type. A profile that needs to report a non-numeric statistic should say what that is rather
462+
* than reopening the map to anything.
458463
*/
459464
case class DataProfile[A](
460465
recordCount: Long,
461466
nullCount: Long,
462467
distinctCount: Long,
463468
schema: DataSchema,
464-
statistics: Map[String, Any])
469+
statistics: Map[String, Double])
465470

466471
/**
467472
* Schema migration for evolution.

‎modules/core/src/main/scala/com/flowforge/core/algebra/EnterpriseTableAlgebra.scala‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import cats.data.{ NonEmptyList, ValidatedNel }
4040
import cats.effect.{ MonadCancel, Resource }
4141
import cats.implicits._
4242
import com.flowforge.core.types.RefinedTypes.{ FieldName, TableName }
43-
import com.flowforge.core.types.{ ErrorCategory, ErrorSeverity, FlowForgeError }
43+
import com.flowforge.core.types.{ ContextValue, ErrorCategory, ErrorSeverity, FlowForgeError }
4444

4545
import java.time.Instant
4646
import scala.concurrent.duration.FiniteDuration
@@ -486,31 +486,31 @@ case class TableNotFound(tableName: TableName) extends TableError {
486486
val message = s"Table '${tableName.value}' not found"
487487
val category = ErrorCategory.System
488488
val severity = ErrorSeverity.Error
489-
val context = Map("tableName" -> tableName.value)
489+
val context = Map("tableName" -> ContextValue.Text(tableName.value))
490490
val cause = None
491491
val timestamp = java.time.Instant.now()
492492
val errorId = java.util.UUID.randomUUID().toString
493493
val isRetryable = false
494494
val recoveryHints = List("Check table name spelling", "Verify table exists", "Check permissions")
495495

496-
def withContext(additionalContext: Map[String, Any]) = this
497-
def withCause(underlyingCause: Throwable) = this
496+
def withContext(additionalContext: Map[String, ContextValue]) = this
497+
def withCause(underlyingCause: Throwable) = this
498498
}
499499

500500
sealed trait PartitionError extends FlowForgeError
501501
case class PartitionNotFound(partitionSpec: PartitionSpec) extends PartitionError {
502502
val message = s"Partition '${partitionSpec.toPartitionPath}' not found"
503503
val category = ErrorCategory.System
504504
val severity = ErrorSeverity.Error
505-
val context = Map("partitionSpec" -> partitionSpec.toPartitionPath)
505+
val context = Map("partitionSpec" -> ContextValue.Text(partitionSpec.toPartitionPath))
506506
val cause = None
507507
val timestamp = java.time.Instant.now()
508508
val errorId = java.util.UUID.randomUUID().toString
509509
val isRetryable = false
510510
val recoveryHints = List("Check partition specification", "Verify partition exists")
511511

512-
def withContext(additionalContext: Map[String, Any]) = this
513-
def withCause(underlyingCause: Throwable) = this
512+
def withContext(additionalContext: Map[String, ContextValue]) = this
513+
def withCause(underlyingCause: Throwable) = this
514514
}
515515

516516
sealed trait BlobError extends FlowForgeError
@@ -519,18 +519,22 @@ case class BlobAccessError(
519519
blobName: String,
520520
reason: String)
521521
extends BlobError {
522-
val message = s"Cannot access blob '$blobName' in bucket '$bucketName': $reason"
523-
val category = ErrorCategory.System
524-
val severity = ErrorSeverity.Error
525-
val context = Map("bucketName" -> bucketName, "blobName" -> blobName, "reason" -> reason)
522+
val message = s"Cannot access blob '$blobName' in bucket '$bucketName': $reason"
523+
val category = ErrorCategory.System
524+
val severity = ErrorSeverity.Error
525+
val context = Map(
526+
"bucketName" -> ContextValue.Text(bucketName),
527+
"blobName" -> ContextValue.Text(blobName),
528+
"reason" -> ContextValue.Text(reason),
529+
)
526530
val cause = None
527531
val timestamp = java.time.Instant.now()
528532
val errorId = java.util.UUID.randomUUID().toString
529533
val isRetryable = true
530534
val recoveryHints = List("Check cloud permissions", "Verify bucket exists", "Retry operation")
531535

532-
def withContext(additionalContext: Map[String, Any]) = this
533-
def withCause(underlyingCause: Throwable) = this
536+
def withContext(additionalContext: Map[String, ContextValue]) = this
537+
def withCause(underlyingCause: Throwable) = this
534538
}
535539

536540
// ===============================

‎modules/core/src/main/scala/com/flowforge/core/algebra/TypeClasses.scala‎

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -753,17 +753,17 @@ object EncodingHints {
753753
*/
754754
sealed trait EncodingError extends FlowForgeError
755755
case class UnsupportedFormat(format: DataFormat, dataType: String) extends EncodingError {
756-
val message = s"Format $format is not supported for data type $dataType"
757-
val category = ErrorCategory.Validation
758-
val severity = ErrorSeverity.Error
759-
val context = Map("format" -> format.toString, "dataType" -> dataType)
760-
val cause = None
756+
val message = s"Format $format is not supported for data type $dataType"
757+
val category = ErrorCategory.Validation
758+
val severity = ErrorSeverity.Error
759+
val context = Map("format" -> ContextValue.Text(format.toString), "dataType" -> ContextValue.Text(dataType))
760+
val cause = None
761761
val timestamp = Instant.now()
762762
val errorId = java.util.UUID.randomUUID().toString
763763
val isRetryable = false
764764
val recoveryHints = List("Use a supported format", "Implement custom encoder")
765765

766-
def withContext(additionalContext: Map[String, Any]): EncodingError =
766+
def withContext(additionalContext: Map[String, ContextValue]): EncodingError =
767767
copy() // Simplified for brevity
768768
def withCause(underlyingCause: Throwable): EncodingError =
769769
copy() // Simplified for brevity
@@ -777,14 +777,14 @@ case class CorruptedData(details: String) extends DecodingError {
777777
val message = s"Data is corrupted: $details"
778778
val category = ErrorCategory.Validation
779779
val severity = ErrorSeverity.Error
780-
val context = Map("details" -> details)
780+
val context = Map("details" -> ContextValue.Text(details))
781781
val cause = None
782782
val timestamp = Instant.now()
783783
val errorId = java.util.UUID.randomUUID().toString
784784
val isRetryable = false
785785
val recoveryHints = List("Check data source", "Re-download data", "Use backup data")
786786

787-
def withContext(additionalContext: Map[String, Any]): DecodingError =
787+
def withContext(additionalContext: Map[String, ContextValue]): DecodingError =
788788
copy() // Simplified for brevity
789789
def withCause(underlyingCause: Throwable): DecodingError =
790790
copy() // Simplified for brevity
@@ -797,16 +797,17 @@ sealed trait SchemaError extends FlowForgeError
797797
case class SchemaIncompatible(expected: DataSchema, actual: DataSchema) extends SchemaError {
798798
val message =
799799
s"Schema incompatible: expected ${expected.fields.length} fields, got ${actual.fields.length}"
800-
val category = ErrorCategory.Validation
801-
val severity = ErrorSeverity.Error
802-
val context = Map("expected" -> expected.toString, "actual" -> actual.toString)
800+
val category = ErrorCategory.Validation
801+
val severity = ErrorSeverity.Error
802+
val context =
803+
Map("expected" -> ContextValue.Text(expected.toString), "actual" -> ContextValue.Text(actual.toString))
803804
val cause = None
804805
val timestamp = Instant.now()
805806
val errorId = java.util.UUID.randomUUID().toString
806807
val isRetryable = false
807808
val recoveryHints = List("Update schema", "Enable schema evolution", "Transform data")
808809

809-
def withContext(additionalContext: Map[String, Any]): SchemaError =
810+
def withContext(additionalContext: Map[String, ContextValue]): SchemaError =
810811
copy() // Simplified for brevity
811812
def withCause(underlyingCause: Throwable): SchemaError =
812813
copy() // Simplified for brevity
@@ -820,14 +821,14 @@ case class SerializationFailed(reason: String) extends SerializationError {
820821
val message = s"Serialization failed: $reason"
821822
val category = ErrorCategory.System
822823
val severity = ErrorSeverity.Error
823-
val context = Map("reason" -> reason)
824+
val context = Map("reason" -> ContextValue.Text(reason))
824825
val cause = None
825826
val timestamp = Instant.now()
826827
val errorId = java.util.UUID.randomUUID().toString
827828
val isRetryable = true
828829
val recoveryHints = List("Retry operation", "Check data format", "Use alternative serializer")
829830

830-
def withContext(additionalContext: Map[String, Any]): SerializationError =
831+
def withContext(additionalContext: Map[String, ContextValue]): SerializationError =
831832
copy() // Simplified for brevity
832833
def withCause(underlyingCause: Throwable): SerializationError =
833834
copy() // Simplified for brevity
@@ -841,14 +842,14 @@ case class RuleViolation(ruleName: String, details: String) extends ContractViol
841842
val message = s"Contract rule '$ruleName' violated: $details"
842843
val category = ErrorCategory.Business
843844
val severity = ErrorSeverity.Error
844-
val context = Map("rule" -> ruleName, "details" -> details)
845+
val context = Map("rule" -> ContextValue.Text(ruleName), "details" -> ContextValue.Text(details))
845846
val cause = None
846847
val timestamp = Instant.now()
847848
val errorId = java.util.UUID.randomUUID().toString
848849
val isRetryable = false
849850
val recoveryHints = List("Fix data quality", "Update contract rules", "Contact data owner")
850851

851-
def withContext(additionalContext: Map[String, Any]): ContractViolation =
852+
def withContext(additionalContext: Map[String, ContextValue]): ContractViolation =
852853
copy() // Simplified for brevity
853854
def withCause(underlyingCause: Throwable): ContractViolation =
854855
copy() // Simplified for brevity

‎modules/core/src/main/scala/com/flowforge/core/patterns/CommonValidations.scala‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,6 @@ package com.flowforge.core.patterns
8383

8484
import cats.syntax.all._
8585
import com.flowforge.core.patterns.ValidationTypes._
86-
import com.flowforge.core.types.RefinedTypes._
8786
import com.flowforge.core.types._
8887

8988
/**
@@ -146,10 +145,22 @@ object CommonValidations {
146145
if (data.nonEmpty) {
147146
valid(data)
148147
} else {
149-
val violation = QualityConstraint.NotNull(
150-
FieldName.unsafeFrom("data"),
148+
// The cast here was hiding a mismatch in the error type, not the value type: this built a
149+
// `QualityConstraint.NotNull`, which is a constraint rather than a violation, so `invalid` produced a
150+
// `ValidatedNel[QualityConstraint, _]` and the cast forced it into the declared result. Reporting an
151+
// actual `QualityViolation` is what the signature promised all along.
152+
//
153+
// `severity` is passed because `QualityViolation` defaults it to `Warning`, while the constraint this
154+
// replaces defaulted to `Error`. This branch rejects the dataset, so anything routing or alerting on
155+
// severity has to see a failure rather than a warning.
156+
invalid(
157+
ValidationError.QualityViolation(
158+
constraint = "nonEmpty",
159+
violatedValue = "data",
160+
message = "Dataset is empty",
161+
severity = ErrorSeverity.Error,
162+
),
151163
)
152-
invalid(violation).asInstanceOf[QualityValidationResult[List[A]]]
153164
}
154165

155166
/**

‎modules/core/src/main/scala/com/flowforge/core/patterns/DataQualityValidation.scala‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,15 @@ object DataQualityValidation {
1616

1717
/**
1818
* Validate data freshness - ensure data is not older than specified duration.
19+
*
20+
* Takes the data it is vouching for and hands it back, the same way [[completeness]] does, so the check
21+
* composes into a validation chain. It used to claim `QualityValidationResult[A]` while having no `A` to
22+
* return, and bridged the gap with `().asInstanceOf[A]`. That threw `ClassCastException` on the success
23+
* path for every `A` except `Unit`, which is why every caller pinned `freshness[Unit]`.
1924
*/
2025
def freshness[A](
2126
fieldName: String,
27+
value: A,
2228
timestamp: Instant,
2329
maxAge: FiniteDuration,
2430
): QualityValidationResult[A] = {
@@ -27,9 +33,7 @@ object DataQualityValidation {
2733
val maxAgeInMillis = maxAge.toMillis
2834

2935
if (age.toMillis <= maxAgeInMillis) {
30-
// Can't return A without having an A - this method signature needs fixing
31-
// For now, return a unit value cast to A as placeholder
32-
().asInstanceOf[A].validNel
36+
value.validNel
3337
} else {
3438
val violation = QualityViolation(
3539
constraint = "freshness",

0 commit comments

Comments
 (0)