Merge pull request #2132 from baoilleach/smiles_charge

Be more strict when parsing charges in SMILES
openbabel · Apr 3, 2020 · d2fedbb · d2fedbb
2 parents 3703358 + 53d07ee
commit d2fedbb
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 10 deletions.
diff --git a/src/formats/smilesformat.cpp b/src/formats/smilesformat.cpp
@@ -1761,19 +1761,37 @@ namespace OpenBabel {
             }
             break;
           case '-':
-            _ptr++;
-            if (!isdigit(*_ptr))
-              charge--;
-            while( isdigit(*_ptr) ) // go number by number
-              charge = charge*10 - ((*_ptr++)-'0');
+            if (charge) {
+              obErrorLog.ThrowError(__FUNCTION__, "Charge can only be specified once", obWarning);
+              return false;
+            }
+            while (*++_ptr == '-')
+              charge--; // handle [O--]
+            if (charge == 0) {
+              while (isdigit(*_ptr)) // handle [O-2]
+                charge = charge * 10 - ((*_ptr++) - '0');
+              if (charge == 0) // handle [Cl-]
+                charge = -1;
+            }
+            else
+              charge--; // finish handling [Ca++]
             _ptr--;
             break;
           case '+':
-            _ptr++;
-            if (!isdigit(*_ptr))
-              charge++;
-            while( isdigit(*_ptr) ) // go number by number
-              charge = charge*10 + ((*_ptr++)-'0');
+            if (charge) {
+              obErrorLog.ThrowError(__FUNCTION__, "Charge can only be specified once", obWarning);
+              return false;
+            }
+            while (*++_ptr == '+')
+              charge++; // handle [Ca++]
+            if (charge == 0) {
+              while (isdigit(*_ptr)) // handle [Ca+2]
+                charge = charge * 10 + ((*_ptr++) - '0');
+              if (charge == 0) // handle [Na+]
+                charge = 1;
+            }
+            else
+              charge++; // finish handling [Ca++]
             _ptr--;
             break;
           case 'H':

diff --git a/test/testbindings.py b/test/testbindings.py
@@ -448,6 +448,30 @@ def testOBMolAssignTotalChargeToAtoms(self):
             mol.OBMol.AssignTotalChargeToAtoms(charge)
             self.assertEqual(mol.write("smi").rstrip(), ans)
 
+    def testParsingChargeInSmiles(self):
+        """Be more strict when parsing charges"""
+        good= [
+                ("[CH3+]", 1),
+                ("[CH2++]", 2),
+                ("[CH2+2]", 2),
+                ("[CH3-]", -1),
+                ("[CH2--]", -2),
+                ("[CH2-2]", -2),
+                ]
+        bad = [
+                "[CH2++2]",
+                "[C+-+-]",
+                "[C+2+]"
+                "[CH2--2]",
+                "[C-+-+]",
+                "[C-2-]"
+                ]
+        for smi, charge in good:
+            mol = pybel.readstring("smi", smi)
+            self.assertEqual(charge, mol.atoms[0].formalcharge)
+        for smi in bad:
+            self.assertRaises(IOError, pybel.readstring, "smi", smi)
+
     def testReadingBenzyne(self):
         """Check that benzyne is read correctly"""
         smi = "c1cccc#c1"