Embed errors in the AST instead of raising

[Burnleydev1]

Closes #10

Hello @panglesd, I'm sorry, I messed up the previous pr and my changes were no longer reflected there when I pushed.

[Burnleydev1]

I converted the function let check_at_least_one_record ~loc rec_flag tds to return the result type but the sites where this function was called below still gave me errors such as,

loc:location ->
rec_flag ->
type_declaration list ->
(unit, Ppxlib.Location.Error.t) result
This expression has type (unit, Ppxlib.Location.Error.t) result
but an expression was expected of type unit
because it is in the left-hand side of a sequence

and the | Ok selection -> on line 543 gave the error
Error (warning 27): unused variable selection.

[Burnleydev1]

This method did not work on check_no_collision as I got this error:
This expression has type Ppxlib.Location.Error.t but an expression was expected of type unit because it is in the result of a conditional with no else branch
this was the code with the changes

    List.iter lbls ~f:(fun { pld_name; pld_loc; _ } ->
      if List.mem generated_funs pld_name.txt ~equal:String.equal
      then
        Error (Location.Error.createf
          ~loc:pld_loc
          "ppx_fields_conv: field name %S conflicts with one of the generated functions"
          pld_name.txt))

[Burnleydev1]

I went through the resources you sent but I am stumped on the application in some functions and where to use a certain method instead of the other.

[panglesd]

Hello @Burnleydev1!

This is already a good start. Let me give some hints/remarks:

• check_at_least_one_record currently checks whether there is no nonrec keyword in the type definitions, and that there is at least one record in the type definitions. So, it could be nice to have to return a list of errors, rather than a (unit, error) result which can only contain one error.
• You have changed the return type of check_at_least_one_record. Before, it was unit, so you could do things such as:

(* has side effects (can raise), but returns unit *)
check_at_least_one_record ...;
(* if we reach this line, there is at least one record *)
rest_of_the_function

However, once check_at_least_one_record is not unit, it (rightfully) becomes an error: the ; separator is used to separate two expression, the first one being of type unit (there is this in the reference manual of OCaml about ; separators): otherwise, what would happen with the return value of the first expression?

So, at the call sites of check_at_least_one_record, you should make sure you handle the return value: maybe, depending on whether it is Ok or Error (or whether the length of the list if it returns a list), you will do different things.

---

Similarly, look at the type of List.iter, it is: 'a list -> ~f:('a -> unit) -> unit. So, it only performs operations which have side effects (they return unit, so they can only return one value (), so they can only do side effects). You cannot use List.iter any more, since there is now a return value!
You should look for other function on list, which are common on functional programming. For instance List.map, List.filter_map, List.fold_left, ... are functions that are very common and useful in OCaml. Using them, you will be able to gather the list of errors that are found when traversing the list of labels.

[Burnleydev1]

Thanks @panglesd for the remarks/tips, I will hop on this.

[Burnleydev1]

Similarly, look at the type of List.iter, it is: 'a list -> ~f:('a -> unit) -> unit. So, it only performs operations which have side effects (they return unit, so they can only return one value (), so they can only do side effects). You cannot use List.iter any more, since there is now a return value!
You should look for other function on list, which are common on functional programming. For instance List.map, List.filter_map, List.fold_left, ... are functions that are very common and useful in OCaml. Using them, you will be able to gather the list of errors that are found when traversing the list of labels.

For this, I used List.filter_map and it worked, I"m trying to fix the errors on the call sites.

[Burnleydev1]

Hi @panglesd, this is an attempt to fix the call site for check_no_collision