Skip to content

Commit

Permalink
Uncovered clause detection
Browse files Browse the repository at this point in the history
Summary:
Add the simplest iteration of detection of uncovered function clauses.

The feature is disabled by default, but can be enabled with `EQWALIZER_CLAUSE_COVERAGE=true`.

For now, it only reports top-level uncovered clauses, without additional information.

Reviewed By: ilya-klyuchnikov

Differential Revision: D56930809

fbshipit-source-id: f781f5e3be4faba19a228d269f4d8c41f9d03f3f
  • Loading branch information
VLanvin authored and facebook-github-bot committed May 3, 2024
1 parent bdeaf92 commit f12166e
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 2 deletions.
6 changes: 6 additions & 0 deletions docs/reference/advanced.md
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,9 @@ a given function or module.

Enable with `EQWALIZER_CHECK_REDUNDANT_GUARDS=true`. With this setting, eqWAlizer will
attempt to detect and report redundant type assertions. See [redundant_guard error](./errors.md#redundant_guard).

### Spec coverage of function clauses

Enable checks of proper coverage of function clauses by specs using `EQWALIZER_CLAUSE_COVERAGE=true`.
By default, eqWAlizer will not check coverage of function clauses by the corresponding spec,
hence a clause may be implicitly left unchecked.
2 changes: 2 additions & 0 deletions eqwalizer/src/main/resources/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,6 @@ eqwalizer {
mode = ${?EQWALIZER_MODE}
error_depth = 4
error_depth = ${?EQWALIZER_ERROR_DEPTH}
clause_coverage = false
clause_coverage = ${?EQWALIZER_CLAUSE_COVERAGE}
}
2 changes: 2 additions & 0 deletions eqwalizer/src/main/scala/com/whatsapp/eqwalizer/package.scala
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ package object eqwalizer {
eqwater: Boolean,
tolerateErrors: Boolean,
checkRedundantGuards: Boolean,
clauseCoverage: Boolean,
mode: Mode.Mode,
errorDepth: Int,
) {
Expand Down Expand Up @@ -76,6 +77,7 @@ package object eqwalizer {
eqwater = config.getBoolean("eqwater"),
tolerateErrors = config.getBoolean("tolerate_errors"),
checkRedundantGuards = config.getBoolean("check_redundant_guards"),
clauseCoverage = config.getBoolean("clause_coverage"),
mode,
errorDepth = config.getInt("error_depth"),
)
Expand Down
15 changes: 13 additions & 2 deletions eqwalizer/src/main/scala/com/whatsapp/eqwalizer/tc/Check.scala
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,17 @@ final class Check(pipelineContext: PipelineContext) {
if (occurrence.eqwater(f.clauses)) {
val clauseEnvs = occurrence.clausesEnvs(f.clauses, ft.argTys, Map.empty)
f.clauses
.lazyZip(1 to f.clauses.length)
.lazyZip(clauseEnvs)
.map((clause, occEnv) => checkClause(clause, argTys, resTy, occEnv, Set.empty))
.foreach((clause, index, occEnv) =>
checkClause(clause, argTys, resTy, occEnv, Set.empty, checkCoverage = (index != f.clauses.length))
)
} else {
f.clauses.map(checkClause(_, argTys, resTy, Env.empty, Set.empty))
f.clauses
.lazyZip(1 to f.clauses.length)
.foreach((clause, index) =>
checkClause(clause, argTys, resTy, Env.empty, Set.empty, checkCoverage = (index != f.clauses.length))
)
}
}

Expand Down Expand Up @@ -96,6 +103,7 @@ final class Check(pipelineContext: PipelineContext) {
resTy: Type,
env0: Env,
exportedVars: Set[String],
checkCoverage: Boolean = false,
): Env = {
val patVars = Vars.clausePatVars(clause)
val env1 = util.enterScope(env0, patVars)
Expand All @@ -105,6 +113,9 @@ final class Check(pipelineContext: PipelineContext) {
typeInfo.setCollect(true)
val (_, env3) = elabPat.elabPats(clause.pats, argTys, env2)
val env4 = elabGuard.elabGuards(clause.guards, env3)
if (pipelineContext.clauseCoverage && checkCoverage && env4.exists { case (_, ty) => subtype.isNoneType(ty) }) {
diagnosticsInfo.add(ClauseNotCovered(clause.pos))
}
val env5 = checkBody(clause.body, resTy, env4)
util.exitScope(env0, env5, exportedVars)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,11 @@ object TcDiagnostics {
def errorName = "ambiguous_union"
override def erroneousExpr: Option[Expr] = Some(expr)
}
case class ClauseNotCovered(pos: Pos) extends TypeError {
override val msg: String = "Clause is not covered by spec"
val errorName = "clause_not_covered"
override val erroneousExpr: Option[Expr] = None
}

implicit val codec: JsonValueCodec[TypeError] = JsonCodecMaker.make(
CodecMakerConfig.withAllowRecursiveTypes(true).withDiscriminatorFieldName(None).withFieldNameMapper {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,5 +70,6 @@ package object tc {
new DiagnosticsInfo()
val errorDepth: Int =
options.errorDepth.getOrElse(config.errorDepth)
val clauseCoverage: Boolean = config.clauseCoverage
}
}
2 changes: 2 additions & 0 deletions eqwalizer/src/test/resources/application.conf
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ eqwalizer {
check_redundant_guards = ${?EQWALIZER_CHECK_REDUNDANT_GUARDS}
mode = standalone
mode = ${?EQWALIZER_MODE}
clause_coverage = false
clause_coverage = ${?EQWALIZER_CLAUSE_COVERAGE}
}

0 comments on commit f12166e

Please sign in to comment.