Merge pull request #81 from upenn-cis1xx/match-fixes

Match Case Improvements

.github/workflows/workflow.yml

@@ -16,6 +16,7 @@ jobs:
           - 4.10.0
           - 4.10.1
           - 4.11.0
+          - 4.11.1
 
     runs-on: ${{ matrix.os }}
 

CHANGELOG.md

@@ -0,0 +1,11 @@
+# Changelog
+
+## 1.6.2 (2021-02-08)
+
+	Match Case Improvements (PR#81)
+
+	* Improved student-facing "fix" for `x :: []` match patterns.
+      Now includes the preferred syntax (`[x]`) that should be used 
+      in place of the above.
+    * Added new tests for the match patterns.
+    * Added CHANGELOG.md (finally)

camelot.opam

@@ -1,6 +1,6 @@
 # This file is generated by dune, edit dune-project instead
 opam-version: "2.0"
-version: "1.4.2"
+version: "1.6.2"
 synopsis: "An OCaml Linter / Style Checker"
 maintainer: ["William Goeller <[email protected]>"]
 authors: ["Vighnesh Vijay" "Daniel Like" "William Goeller"]

dune-project

@@ -2,7 +2,7 @@
 (name camelot)
 (generate_opam_files true)
 
-(version 1.4.2)
+(version 1.6.2)
 
 (source (github upenn-cis1xx/camelot))
 (license "Apache License 2.0")

lib/style/match.ml

@@ -105,14 +105,16 @@ module MatchListVerbose : EXPRCHECK = struct
   let check = make_check 
       (pat_pred)
       (fun location source pattern st -> 
-         (* Regexp that matches literal(0 or more spaces)::(0 or more space)[] *)
-         let matcher = Str.regexp "[a-zA-Z0-9_]+[ ]*::[ ]*\\[\\]" in
-         let test s = try Str.search_forward matcher s 0 >= 0 with _ -> false in
-         if pat_pred pattern then
-           let refined_loc = Warn.warn_loc_of_loc location.file pattern.ppat_loc in
-           let raw_source = IOUtils.code_at_loc refined_loc source in
-           if test raw_source then
-             st := Hint.mk_hint refined_loc ("| " ^ raw_source ^ " -> ...") fix violation :: !st 
+        (* Regexp that matches literal(0 or more spaces)::(0 or more space)[] *)
+        let matcher = Str.regexp "\\([a-zA-Z0-9_]+\\)[ ]*::[ ]*\\[\\]" in
+        let test s = try Str.search_forward matcher s 0 >= 0 with _ -> false in
+        if pat_pred pattern then
+          let refined_loc = Warn.warn_loc_of_loc location.file pattern.ppat_loc in
+          let raw_source = IOUtils.code_at_loc refined_loc source in
+          if test raw_source then
+            let literal = Str.matched_group 1 raw_source in
+            let fix = "expressing this match case more compactly, such as: | [" ^ literal ^ "] -> ..." in
+            st := Hint.mk_hint refined_loc ("| " ^ raw_source ^ " -> ...") fix violation :: !st
       )
       Int.max_int
       true

test/examples/match.ml

@@ -45,6 +45,7 @@ let b l =
 let b l =
   match l with
   | x :: [] -> ()
+  | abc :: [] -> ()
   | x :: y :: [] -> ()
   | _ -> ()
 
@@ -53,6 +54,7 @@ let b l =
   match l with
   | [x] -> ()
   | [x; y] -> ()
+  | [_] -> ()
   | _ -> ()
 
 

test/test.ml

@@ -393,43 +393,43 @@ let%expect_test _ =
   lint_and_hint to_lint;
   [%expect{|
     (* ------------------------------------------------------------------------ *)
-    File ./examples/match.ml, line 110, columns: 13-20
+    File ./examples/match.ml, line 112, columns: 13-20
     Warning:
     	using an overly complex match clause
     You wrote:
     	 | y :: [] -> ...
     Consider:
-    	expressing this match case more compactly
+    	expressing this match case more compactly, such as: | [y] -> ...
 
     (* ------------------------------------------------------------------------ *)
-    File ./examples/match.ml, line 110, columns: 4-11
+    File ./examples/match.ml, line 112, columns: 4-11
     Warning:
     	using an overly complex match clause
     You wrote:
     	 | x :: [] -> ...
     Consider:
-    	expressing this match case more compactly
+    	expressing this match case more compactly, such as: | [x] -> ...
 
     (* ------------------------------------------------------------------------ *)
-    File ./examples/match.ml, line 104, columns: 7-14
+    File ./examples/match.ml, line 106, columns: 7-14
     Warning:
     	using an overly complex match clause
     You wrote:
     	 | x :: [] -> ...
     Consider:
-    	expressing this match case more compactly
+    	expressing this match case more compactly, such as: | [x] -> ...
 
     (* ------------------------------------------------------------------------ *)
-    File ./examples/match.ml, line 98, columns: 4-11
+    File ./examples/match.ml, line 100, columns: 4-11
     Warning:
     	using an overly complex match clause
     You wrote:
     	 | x :: [] -> ...
     Consider:
-    	expressing this match case more compactly
+    	expressing this match case more compactly, such as: | [x] -> ...
 
     (* ------------------------------------------------------------------------ *)
-    File ./examples/match.ml, lines 79-80, columns: 2-15
+    File ./examples/match.ml, lines 81-82, columns: 2-15
     Warning:
     	using pattern matching on a tuple (for fewer than 2 cases)
     You wrote:
@@ -438,7 +438,7 @@ let%expect_test _ =
     	using a let statement to extract tuple fields
 
     (* ------------------------------------------------------------------------ *)
-    File ./examples/match.ml, lines 67-69, columns: 2-16
+    File ./examples/match.ml, lines 69-71, columns: 2-16
     Warning:
     	using pattern matching on a record (for fewer than 3 cases)
     You wrote:
@@ -447,22 +447,31 @@ let%expect_test _ =
     	using a let statement to extract record fields
 
     (* ------------------------------------------------------------------------ *)
-    File ./examples/match.ml, lines 63-64, columns: 2-16
+    File ./examples/match.ml, lines 65-66, columns: 2-16
     Warning:
     	using pattern matching on a record (for fewer than 3 cases)
     You wrote:
     	 match r with | { x; y } -> ()
     Consider:
     	using a let statement to extract record fields
 
+    (* ------------------------------------------------------------------------ *)
+    File ./examples/match.ml, line 48, columns: 4-13
+    Warning:
+    	using an overly complex match clause
+    You wrote:
+    	 | abc :: [] -> ...
+    Consider:
+    	expressing this match case more compactly, such as: | [abc] -> ...
+
     (* ------------------------------------------------------------------------ *)
     File ./examples/match.ml, line 47, columns: 4-11
     Warning:
     	using an overly complex match clause
     You wrote:
     	 | x :: [] -> ...
     Consider:
-    	expressing this match case more compactly
+    	expressing this match case more compactly, such as: | [x] -> ...
 
     (* ------------------------------------------------------------------------ *)
     File ./examples/match.ml, line 42, columns: 4-11
@@ -471,7 +480,7 @@ let%expect_test _ =
     You wrote:
     	 | _ :: [] -> ...
     Consider:
-    	expressing this match case more compactly
+    	expressing this match case more compactly, such as: | [_] -> ...
 
     (* ------------------------------------------------------------------------ *)
     File ./examples/match.ml, lines 12-15, columns: 15-5