Skip to content

Commit

Permalink
Access error messages from SMARTS parser.
Browse files Browse the repository at this point in the history
  • Loading branch information
@johnmay
johnmay committed Oct 9, 2018
1 parent 4d368ac commit 8103e08
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 24 deletions.
109 changes: 95 additions & 14 deletions tool/smarts/src/main/java/org/openscience/cdk/isomorphism/Smarts.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,62 @@ public final class Smarts {
private static final String BSTEREO_UPU = "/?";
private static final String BSTEREO_DNU = "\\?";


private static final class SmartsError {
private String str;
private int pos;
private String mesg;

public SmartsError(String str, int pos, String mesg) {
this.str = str;
this.pos = pos;
this.mesg = mesg;
}
}

public static ThreadLocal<SmartsError> lastError = new ThreadLocal<>();

private static void setErrorMesg(String sma, int pos, String str) {
lastError.set(new SmartsError(sma, pos, str));
}

/**
* Access the error message from previously parsed SMARTS (when
* {@link #parse)=false).
*
* @return the error message, or null if none
*/
public static String getLastErrorMesg() {
SmartsError error = lastError.get();
if (error != null)
return error.mesg;
return null;
}

/**
* Access a display of the error position from previously parsed SMARTS
* (when {@link #parse)=false).
*
* @return the error message, or null if none
*/
public static String getLastErrorLocation() {
SmartsError error = lastError.get();
if (error != null) {
StringBuilder sb = new StringBuilder();
sb.append(error.str);
sb.append('\n');
char[] cs = new char[error.pos-1];
Arrays.fill(cs, ' ');
sb.append(cs);
sb.append('^');
sb.append('\n');
return sb.toString();
}
return null;
}

private static final class Parser {
public String error;
private String str;
private IAtomContainer mol;
private int flav;
Expand Down Expand Up @@ -495,8 +550,6 @@ else if (isFlavor(FLAVOR_CACTVS))
break;
}
break;
case 'J':
return false;
case 'K':
switch (next()) {
case 'r': // Kr=Krypton
Expand Down Expand Up @@ -1232,7 +1285,8 @@ private boolean parseBondExpr(Expr dest, IBond bond, char lastOp) {
}

private void unget() {
pos--;
if (pos <= str.length())
pos--;
}

private boolean hasPrecedence(char lastOp, char currOp) {
Expand All @@ -1247,16 +1301,23 @@ private boolean parseAtomExpr() {
QueryAtom atom = new QueryAtom(mol.getBuilder());
Expr expr = new Expr(Expr.Type.NONE);
atom.setExpression(expr);
if (!parseExplicitHydrogen(atom, expr) && !parseAtomExpr(atom, expr, '\0'))
if (!parseExplicitHydrogen(atom, expr) &&
!parseAtomExpr(atom, expr, '\0')) {
error = "Invalid atom expression";
return false;
}
append(atom);
return true;
}

boolean parseBondExpr() {
bond = new QueryBond(mol.getBuilder());
bond.setExpression(new Expr(Expr.Type.NONE));
return parseBondExpr(bond.getExpression(), bond, '\0');
if (!parseBondExpr(bond.getExpression(), bond, '\0')) {
error = "Invalid bond expression";
return false;
}
return true;
}

void newFragment() {
Expand All @@ -1270,22 +1331,28 @@ boolean begComponentGroup() {

boolean endComponentGroup() {
// closing an unopen component group
if (curComponentId == 0)
if (curComponentId == 0) {
error = "Closing unopened component grouping";
return false;
}
curComponentId = 0;
return true;
}

boolean openBranch() {
if (prev == null || bond != null)
if (prev == null || bond != null) {
error = "No previous atom to open branch";
return false;
}
stack.push(prev);
return true;
}

boolean closeBranch() {
if (stack.isEmpty() || bond != null)
if (stack.isEmpty() || bond != null) {
error = "Closing unopened branch";
return false;
}
prev = stack.pop();
return true;
}
Expand All @@ -1311,8 +1378,10 @@ boolean closeRing(int rnum) {
Expr closeExpr = ((QueryBond) BondRef.deref(this.bond)).getExpression();
if (openExpr == null)
((QueryBond) BondRef.deref(bond)).setExpression(closeExpr);
else if (!openExpr.equals(closeExpr))
else if (!openExpr.equals(closeExpr)) {
error = "Open/close expressions are not equivalent";
return false;
}
this.bond = null;
} else if (openExpr == null) {
((QueryBond) BondRef.deref(bond)).setExpression(new Expr(SINGLE_OR_AROMATIC));
Expand Down Expand Up @@ -1453,11 +1522,15 @@ else if (sub2 != null)
// final check
boolean finish() {
// check for unclosed rings, components, and branches
if (numRingOpens != 0 || curComponentId != 0 || !stack.isEmpty())
if (numRingOpens != 0 || curComponentId != 0 || !stack.isEmpty()) {
error = "Unclosed ring, component group, or branch";
return false;
}
if (role != ReactionRole.None) {
if (role != ReactionRole.Agent)
if (role != ReactionRole.Agent) {
error = "Missing '>' to complete reaction";
return false;
}
markReactionRoles();
for (IAtom atom : mol.atoms()) {
ReactionRole role = atom.getProperty(CDKConstants.REACTION_ROLE);
Expand Down Expand Up @@ -1573,7 +1646,10 @@ private char peek() {
}

private char next() {
return pos < str.length() ? str.charAt(pos++) : '\0';
if (pos < str.length())
return str.charAt(pos++);
pos++;
return '\0';
}

private static boolean isDigit(char c) {
Expand Down Expand Up @@ -1750,6 +1826,7 @@ public boolean parse() {
return finish();

default:
error = "Unexpected character";
return false;
}
}
Expand All @@ -1764,7 +1841,7 @@ else if (role == ReactionRole.Reactant)
else if (role == ReactionRole.Agent)
role = ReactionRole.Product;
else
return false;
error = "To many '>' in reaction";
int idx = mol.getAtomCount() - 1;
while (idx >= 0) {
IAtom atom = mol.getAtom(idx--);
Expand Down Expand Up @@ -1923,7 +2000,11 @@ private static boolean generateBond(StringBuilder sb, Expr expr) {
*/
public static boolean parse(IAtomContainer mol, String smarts, int flavor) {
Parser state = new Parser(mol, smarts, flavor);
return state.parse();
if (!state.parse()) {
setErrorMesg(smarts, state.pos, state.error);
return false;
}
return true;
}

/**
Expand Down
23 changes: 13 additions & 10 deletions tool/smarts/src/test/java/org/openscience/cdk/isomorphism/ParserTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,14 @@
*/
package org.openscience.cdk.isomorphism;

import org.junit.Assert;
import org.hamcrest.CoreMatchers;
import org.junit.Test;
import org.openscience.cdk.CDKTestCase;
import org.openscience.cdk.DefaultChemObjectBuilder;
import org.openscience.cdk.exception.CDKException;
import org.openscience.cdk.interfaces.IBond;
import org.openscience.cdk.interfaces.IChemObjectBuilder;
import org.openscience.cdk.isomorphism.matchers.QueryAtomContainer;
import org.openscience.cdk.isomorphism.matchers.smarts.AnyOrderQueryBond;
import org.openscience.cdk.isomorphism.matchers.smarts.AromaticQueryBond;
import org.openscience.cdk.isomorphism.matchers.smarts.OrderQueryBond;
import org.openscience.cdk.isomorphism.matchers.smarts.SMARTSAtom;
import org.openscience.cdk.silent.SilentChemObjectBuilder;

import static org.junit.Assert.assertThat;

/**
* JUnit test routines for the SMARTS parser.
*
Expand All @@ -44,13 +38,22 @@ public class ParserTest extends CDKTestCase {
private void parse(String smarts, int flav) throws Exception {
IChemObjectBuilder builder = SilentChemObjectBuilder.getInstance();
if (!Smarts.parse(builder.newAtomContainer(), smarts, flav))
throw new Exception("Invalid SMARTS!");
throw new Exception(Smarts.getLastErrorMesg());
}

private void parse(String smarts) throws Exception {
parse(smarts, Smarts.FLAVOR_LOOSE);
}

@Test
public void errorHandling() {
IChemObjectBuilder builder = SilentChemObjectBuilder.getInstance();
if (!Smarts.parse(builder.newAtomContainer(), "CCCJCCC")) {
assertThat(Smarts.getLastErrorMesg(), CoreMatchers.is("Unexpected character"));
assertThat(Smarts.getLastErrorLocation(), CoreMatchers.is("CCCJCCC\n ^\n"));
}
}

@Test
public void testQueryAtomCreation() throws Exception {
parse("*");
Expand Down

0 comments on commit 8103e08

Please sign in to comment.