From e540755436ec9ca39a9fc9d624124d2cbdd7b653 Mon Sep 17 00:00:00 2001
From: Gabor Csardi <csardi.gabor@gmail.com>
Date: Thu, 12 May 2016 16:26:29 +0100
Subject: [PATCH] trailing_semicolon_linter, #147

---
 NAMESPACE                                     |  1 +
 NEWS.md                                       |  1 +
 R/trailing_semicolons_linter.R                | 39 +++++++++++++++++++
 man/linters.Rd                                |  7 +++-
 .../test-trailing_semicolons_linter.R         | 23 +++++++++++
 5 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100644 R/trailing_semicolons_linter.R
 create mode 100644 tests/testthat/test-trailing_semicolons_linter.R

diff --git a/NAMESPACE b/NAMESPACE
index 6a098e07d..cc5d2131d 100644
--- a/NAMESPACE
+++ b/NAMESPACE
@@ -31,6 +31,7 @@ export(snake_case_linter)
 export(spaces_inside_linter)
 export(spaces_left_parentheses_linter)
 export(trailing_blank_lines_linter)
+export(trailing_semicolons_linter)
 export(trailing_whitespace_linter)
 export(with_defaults)
 import(rex)
diff --git a/NEWS.md b/NEWS.md
index 8b78839dc..f5bf7ba31 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,4 +1,5 @@
 # lintr 1.0.0.9000 #
+* trailing_semicolon_linter (#147, @gaborcsardi)
 
 * Commas linter handles missing arguments calls properly (#145)
 
diff --git a/R/trailing_semicolons_linter.R b/R/trailing_semicolons_linter.R
new file mode 100644
index 000000000..ecfcdb034
--- /dev/null
+++ b/R/trailing_semicolons_linter.R
@@ -0,0 +1,39 @@
+#' @describeIn linters check there are no trailing semicolons.
+#' @export
+trailing_semicolons_linter <- function(source_file) {
+
+  allsc <- which(source_file$parsed_content$token == "';'")
+
+  if (!length(allsc)) return(list())
+
+  ## Check that it is the last token in the line
+  ## Note that we need to restrict the search to terminal
+  ## nodes, because parse is buggy and sometimes reports false,
+  ## too large end positions for non-terminal nodes.
+  badsc <- Filter(
+    x = allsc,
+    f = function(line) {
+      with(
+        source_file$parsed_content,
+        all(! terminal | line1 != line1[line] | col2 <= col2[line])
+      )
+    }
+  )
+
+  lapply(
+    badsc,
+    function(line) {
+      parsed <- source_file$parsed_content[line, ]
+      Lint(
+        filename = source_file$filename,
+        line_number = parsed$line1,
+        column_number = parsed$col1,
+        type = "style",
+        message = "Avoid trailing semicolons, they are not needed.",
+        line = source_file$lines[as.character(parsed$line1)],
+        ranges = list(c(parsed$col1, parsed$col2)),
+        linter = "trailing_semicolons_linter"
+      )
+    }
+  )
+}
diff --git a/man/linters.Rd b/man/linters.Rd
index 82a343efa..5287ffeb1 100644
--- a/man/linters.Rd
+++ b/man/linters.Rd
@@ -1,5 +1,5 @@
 % Generated by roxygen2: do not edit by hand
-% Please edit documentation in R/absolute_paths_linter.R, R/assignment_linter.R, R/closed_curly_linter.R, R/commas_linter.R, R/commented_code_linter.R, R/infix_spaces_linter.R, R/line_length_linter.R, R/no_tab_linter.R, R/object_usage_linter.R, R/objects_linter.R, R/open_curly_linter.R, R/single_quotes_linter.R, R/spaces_inside_linter.R, R/spaces_left_parentheses_linter.R, R/trailing_blank_lines_linter.R, R/trailing_whitespace_linter.R, R/zzz.R
+% Please edit documentation in R/absolute_paths_linter.R, R/assignment_linter.R, R/closed_curly_linter.R, R/commas_linter.R, R/commented_code_linter.R, R/infix_spaces_linter.R, R/line_length_linter.R, R/no_tab_linter.R, R/object_usage_linter.R, R/objects_linter.R, R/open_curly_linter.R, R/single_quotes_linter.R, R/spaces_inside_linter.R, R/spaces_left_parentheses_linter.R, R/trailing_blank_lines_linter.R, R/trailing_semicolons_linter.R, R/trailing_whitespace_linter.R, R/zzz.R
 \name{absolute_paths_linter}
 \alias{absolute_paths_linter}
 \alias{assignment_linter}
@@ -20,6 +20,7 @@
 \alias{spaces_inside_linter}
 \alias{spaces_left_parentheses_linter}
 \alias{trailing_blank_lines_linter}
+\alias{trailing_semicolons_linter}
 \alias{trailing_whitespace_linter}
 \title{linters}
 \usage{
@@ -59,6 +60,8 @@ spaces_left_parentheses_linter(source_file)
 
 trailing_blank_lines_linter(source_file)
 
+trailing_semicolons_linter(source_file)
+
 trailing_whitespace_linter(source_file)
 }
 \arguments{
@@ -121,6 +124,8 @@ unless they are in a function call.
 
 \item \code{trailing_blank_lines_linter}: check there are no trailing blank lines.
 
+\item \code{trailing_semicolons_linter}: check there are no trailing semicolons.
+
 \item \code{trailing_whitespace_linter}: check there are no trailing whitespace characters.
 }}
 
diff --git a/tests/testthat/test-trailing_semicolons_linter.R b/tests/testthat/test-trailing_semicolons_linter.R
new file mode 100644
index 000000000..032ed4643
--- /dev/null
+++ b/tests/testthat/test-trailing_semicolons_linter.R
@@ -0,0 +1,23 @@
+context("trailing_semicolons_linter")
+
+test_that("finds trailing semicolons", {
+  expect_lint("function() { 42 }\n",
+    NULL,
+    trailing_semicolons_linter)
+
+  expect_lint("function() { \"But here\";\n\"  and here\"; }",
+    rex("trailing semicolons"),
+    trailing_semicolons_linter)
+})
+
+test_that("finds trailing semicolons even with trailing whitespace", {
+  expect_lint("function() { \"Here, too!\";  \n }\n",
+    rex("trailing semicolons"),
+    trailing_semicolons_linter)
+})
+
+test_that("ignores non-trailing semicolons", {
+  expect_lint("function() { 1 ; 2 ; \"is good\" }\n",
+    NULL,
+    trailing_semicolons_linter)
+})