En-passant erroneously rejected as invalid when specified via SAN

[minor-fixes]

With 3.2.0, the following case prints test successful:

use chess::{Board, BoardStatus, ChessMove, Square};
let mut board = Board::default();

assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "e4").expect("invalid: e4"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "e5").expect("invalid: e5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "d4").expect("invalid: d4"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "h5").expect("invalid: h5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "d5").expect("invalid: d5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "c5").expect("invalid: c5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::new(Square::D5, Square::C6, None)); // en passant
assert_eq!(board.status(), BoardStatus::Ongoing);

println!("test successful");

whereas this case panics with invalid: dxc6: InvalidSanMove:

use chess::{Board, BoardStatus, ChessMove, Square};
let mut board = Board::default();

assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "e4").expect("invalid: e4"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "e5").expect("invalid: e5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "d4").expect("invalid: d4"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "h5").expect("invalid: h5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "d5").expect("invalid: d5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "c5").expect("invalid: c5"));
assert_eq!(board.status(), BoardStatus::Ongoing);
board = board.make_move_new(ChessMove::from_san(&board, "dxc6").expect("invalid: dxc6")); // same en passant!
assert_eq!(board.status(), BoardStatus::Ongoing);

println!("test successful");

As far as I can tell, this is the same move, and valid SAN? (SAN comes from the beginning of https://www.chess.com/game/live/9695070671)

[minor-fixes]

Upon further testing, it looks like the first case works because moves generated via ChessMove::new() are not validated to be legal, while ones from SAN are.

The SAN en passant move maybe is failing because the SAN does not have the e.p. suffix: https://docs.rs/chess/3.2.0/src/chess/chess_move.rs.html#310 and thus it is not recognized as en passant. Though, maybe it should, since Wikipedia says this suffix is optional: https://en.wikipedia.org/wiki/Algebraic_notation_(chess)#Notation_for_moves

[minor-fixes]

Examining this loop a bit more, where the parsed SAN is being checked against the full list of legal moves - if the SAN is assumed to be valid (say, SAN from another system that is being played back, which happens to be what I'm trying to do) then checking for takes here and here is redundant, since the list of legal moves with a given source and destination will either be a capture or non-capture; it's not possible to have both capture and non-capture returned as legal moves, so there's no need to do these checks to disambiguate between the two.

However, dropping these checks would make the parsing more permissive to nonsensical moves, such as Nxf3 being accepted and matching to Nf3 when there is no capture on f3; this sanitization is admittedly useful if the SAN is possibly invalid (say, allowing a user to input moves during a game).

Some possible solutions are:

• drop the checks, and fail to reject SAN capture strings that are actually valid non-capture moves, in order to handle non-annotated en passant
• have a second function that assumes valid PGN, that does fewer checks and can handle non-annotated en passant as a result
• store whether a move is a capture or not in the ChessMove struct returned by the MoveGen, and compare this to whether the input SAN declared a capture. Possibly the most robust, but a breaking change for ChessMove?

[Philipp-Sc]

Chess.js generates the following moves:

c4 g6 e4 d6 Nc3 c5 Nge2 e5 Nd5 Bg7 Nec3 Nf6 d3 Nc6 Bg5 Nd4 Be2 Be6 O-O h6 Bh4 O-O a4 Bxd5 cxd5 a6 a5 Rb8 Ra2 b5 axb6

https://lichess.org/NlJw2ipp#31

axb6 is an en_passant move and this is the standard notation for it. It being rejected as invalid is definitely a bug.

EDIT: Chessjs provides a flag:
'e' - an en passant capture

this can be used to add the necessary suffix " e.p."