When ring membership is specified without a number, match any ring at…

…om. Resolves bug 1168.

Change-Id: I3828029dcb4de440af630a30e1946d5438bab9e7
Signed-off-by: Egon Willighagen <egonw@users.sourceforge.net>

src/main/org/openscience/cdk/isomorphism/matchers/smarts/RingMembershipAtom.java

@@ -32,12 +32,20 @@
  */
 public class RingMembershipAtom extends SMARTSAtom {
 	private static final long serialVersionUID = -7963168231557641862L;
-	/**
-	 * Number of SSSR
-	 */
+
+    /** Number of rings to which this atom belongs, if < 0 check any ring membership. */
 	private int numSSSR;
 
-	public RingMembershipAtom(int num) {
+    /**
+     * Ring membership query atom. Check if the an atom belongs to <i>num</i> of
+     * rings. To specify any ring membership, <i>num</i> should be specified as
+     * < 0. Generally in SMARTS it's better negate ring membership with {@code
+     * [!R]} however for legacy reasons {@code [R0]} was accepted and checks
+     * this atoms belongs to 0 rings.
+     *
+     * @param num number of rings which this atom belongs to, < 0 any ring.
+     */
+    public RingMembershipAtom(int num) {
 		this.numSSSR = num;
 	}
 
@@ -47,7 +55,8 @@ public RingMembershipAtom(int num) {
 	public boolean matches(IAtom atom) {
 		if (atom.getFlag(CDKConstants.ISINRING)) {
 			IRingSet ringSet = (IRingSet)atom.getProperty(CDKConstants.SMALLEST_RINGS);
-			return ringSet.getAtomContainerCount() == numSSSR;
+            // < 0 means any ring, as you can see below R0 is valid
+			return numSSSR < 0 || ringSet.getAtomContainerCount() == numSSSR;
 		} else {
             if (numSSSR == 0) return true;
         }

src/main/org/openscience/cdk/smiles/smarts/parser/SMARTSParser.jjt

@@ -686,7 +686,7 @@ void Valence() #Valence :
 void RingMembership() #RingMembership : 
 { StringBuffer digits = new StringBuffer(); }
 {
-    <R> { jjtThis.setNumOfMembership(1); } 
+    <R> { jjtThis.setNumOfMembership(-1); }
     [ ( <DIGIT> { digits.append(token.image); } )+ 
     { jjtThis.setNumOfMembership( Integer.parseInt(digits.toString()) ); } ]
 }

src/test/org/openscience/cdk/smiles/smarts/parser/SMARTSSearchTest.java

@@ -44,6 +44,9 @@
 import java.io.InputStream;
 import java.util.List;
 
+import static org.hamcrest.CoreMatchers.is;
+import static org.junit.Assert.assertThat;
+
 /**
  * JUnit test routines for the SMARTS substructure search.
  *
@@ -1659,5 +1662,16 @@ public void testBondOrderQueryKekuleVsSmiles() throws Exception {
         Assert.assertEquals(0, results[0]);
         Assert.assertEquals(0, results[1]);
     }
+
+    /**
+     * Checks that when no number is specified for ring member ship any ring
+     * atom is matched.
+     *
+     * @cdk.bug 1168
+     */
+    @Test public void unspecifiedRingMembership() throws Exception {
+        assertThat(match("[#6+0&R]=[#6+0&!R]", "C1=C2CCCC2CCC1"),
+                   is(new int[]{0, 0}));
+    }
 }