Clean up the validation of .Pp, .PP, .sp, and .br. Make sure all

combinations are handled, and are handled in a systematic manner.
This resolves some erratic duplicate handling, handles a number of
missing cases, and improves diagnostics in various respects.

Move validation of .br and .sp to the roff validation module
rather than doing that twice in the mdoc and man validation modules.
Move the node relinking function to the roff library where it belongs.

In validation functions, only look at the node itself, at previous
nodes, and at descendants, not at following nodes or ancestors,
such that only nodes are inspected which are already validated.

1 files changed, 48 insertions, 39 deletions

diff --git a/man_validate.c b/man_validate.c
index 78a1fcd4..806a6cdc 100644
--- a/man_validate.c
+++ b/man_validate.c
@@ -49,16 +49,16 @@ static	void	  check_text(CHKARGS);
 static	void	  post_AT(CHKARGS);
 static	void	  post_IP(CHKARGS);
 static	void	  post_OP(CHKARGS);
+static	void	  post_SH(CHKARGS);
 static	void	  post_TH(CHKARGS);
 static	void	  post_UC(CHKARGS);
 static	void	  post_UR(CHKARGS);
 static	void	  post_in(CHKARGS);
-static	void	  post_vs(CHKARGS);
 
 static	const v_check man_valids[MAN_MAX - MAN_TH] = {
 	post_TH,    /* TH */
-	NULL,       /* SH */
-	NULL,       /* SS */
+	post_SH,    /* SH */
+	post_SH,    /* SS */
 	NULL,       /* TP */
 	NULL,       /* TQ */
 	check_abort,/* LP */
@@ -151,15 +151,7 @@ man_node_validate(struct roff_man *man)
 		break;
 	default:
 		if (n->tok < ROFF_MAX) {
-			switch (n->tok) {
-			case ROFF_br:
-			case ROFF_sp:
-				post_vs(man, n);
-				break;
-			default:
-				roff_validate(man);
-				break;
-			}
+			roff_validate(man);
 			break;
 		}
 		assert(n->tok >= MAN_TH && n->tok < MAN_MAX);
@@ -240,6 +232,42 @@ post_OP(CHKARGS)
 }
 
 static void
+post_SH(CHKARGS)
+{
+	struct roff_node	*nc;
+
+	if (n->type != ROFFT_BODY || (nc = n->child) == NULL)
+		return;
+
+	if (nc->tok == MAN_PP && nc->body->child != NULL) {
+		while (nc->body->last != NULL) {
+			man->next = ROFF_NEXT_CHILD;
+			roff_node_relink(man, nc->body->last);
+			man->last = n;
+		}
+	}
+
+	if (nc->tok == MAN_PP || nc->tok == ROFF_sp || nc->tok == ROFF_br) {
+		mandoc_vmsg(MANDOCERR_PAR_SKIP, man->parse,
+		    nc->line, nc->pos, "%s after %s",
+		    roff_name[nc->tok], roff_name[n->tok]);
+		roff_node_delete(man, nc);
+	}
+
+	/*
+	 * Trailing PP is empty, so it is deleted by check_par().
+	 * Trailing sp is significant.
+	 */
+
+	if ((nc = n->last) != NULL && nc->tok == ROFF_br) {
+		mandoc_vmsg(MANDOCERR_PAR_SKIP, man->parse,
+		    nc->line, nc->pos, "%s at the end of %s",
+		    roff_name[nc->tok], roff_name[n->tok]);
+		roff_node_delete(man, nc);
+	}
+}
+
+static void
 post_UR(CHKARGS)
 {
 	if (n->type == ROFFT_HEAD && n->child == NULL)
@@ -267,6 +295,14 @@ check_par(CHKARGS)
 			roff_node_delete(man, n);
 		break;
 	case ROFFT_BODY:
+		if (n->child != NULL &&
+		    (n->child->tok == ROFF_sp || n->child->tok == ROFF_br)) {
+			mandoc_vmsg(MANDOCERR_PAR_SKIP,
+			    man->parse, n->child->line, n->child->pos,
+			    "%s after %s", roff_name[n->child->tok],
+			    roff_name[n->tok]);
+			roff_node_delete(man, n->child);
+		}
 		if (n->child == NULL)
 			mandoc_vmsg(MANDOCERR_PAR_SKIP,
 			    man->parse, n->line, n->pos,
@@ -494,30 +530,3 @@ post_in(CHKARGS)
 	free(n->child->string);
 	n->child->string = s;
 }
-
-static void
-post_vs(CHKARGS)
-{
-
-	if (NULL != n->prev)
-		return;
-
-	switch (n->parent->tok) {
-	case MAN_SH:
-	case MAN_SS:
-	case MAN_PP:
-		mandoc_vmsg(MANDOCERR_PAR_SKIP, man->parse, n->line, n->pos,
-		    "%s after %s", roff_name[n->tok],
-		    roff_name[n->parent->tok]);
-		/* FALLTHROUGH */
-	case TOKEN_NONE:
-		/*
-		 * Don't warn about this because it occurs in pod2man
-		 * and would cause considerable (unfixable) warnage.
-		 */
-		roff_node_delete(man, n);
-		break;
-	default:
-		break;
-	}
-}