Bug 566107 - Option parsers can fail to trim extra characters when using patterns with numbered back-references

Clarify in the javadoc that numbered back-references should not be used
in order for option pattern to safely work.  Add a unit test
demonstrating the flaw when numbered back-references are used.  Update
GCC parser as an example for not using numbered back-references.

Change-Id: I008b3589486dc9fb0d9d9aa41f7cc2443bf4351e
Signed-off-by: Marc-Andre Laperle <malaperle@gmail.com>

build/org.eclipse.cdt.managedbuilder.core.tests/tests/org/eclipse/cdt/managedbuilder/language/settings/providers/tests/GCCBuildCommandParserTest.java

@@ -20,6 +20,8 @@ import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import org.eclipse.cdt.core.ErrorParserManager;
 import org.eclipse.cdt.core.dom.ast.gnu.cpp.GPPLanguage;
@@ -406,6 +408,59 @@ public class GCCBuildCommandParserTest extends BaseTestCase {
 		assertEquals(expected, entries.get(0));
 	}
 
+	/**
+	 * Test that an option string containing extra characters after its pattern (often the source file name) gets trimmed properly.
+	 */
+	public void testAbstractBuildCommandParser_FileNameRemovalInOption() throws Exception {
+		IProject project = ResourceHelper.createCDTProjectWithConfig(getName());
+		ICConfigurationDescription[] cfgDescriptions = getConfigurationDescriptions(project);
+		ICConfigurationDescription cfgDescription = cfgDescriptions[0];
+
+		final IFile file = ResourceHelper.createFile(project, "file.cpp");
+		final IFile file2 = ResourceHelper.createFile(project, "file2.cpp");
+
+		AbstractBuildCommandParser parser = new MockBuildCommandParser() {
+			final Pattern GENERIC_OPTION_PATTERN = Pattern.compile("-.*");
+
+			@Override
+			protected AbstractOptionParser[] getOptionParsers() {
+				return new AbstractOptionParser[] { new IncludePathOptionParser("-Isimple", "simple"),
+						new IncludePathOptionParser("-I(arg)withbackreference\\1", "willnotwork"),
+						new IncludePathOptionParser("-I(?<mygroup>arg)withbackreference\\k<mygroup>",
+								"backreferenceworks") };
+			}
+
+			@Override
+			protected List<String> parseOptions(String line) {
+				List<String> options = new ArrayList<>();
+				Matcher optionMatcher = GENERIC_OPTION_PATTERN.matcher(line);
+				while (optionMatcher.find()) {
+					String option = optionMatcher.group(0);
+					if (option != null) {
+						options.add(option);
+					}
+				}
+				return options;
+			}
+		};
+		parser.startup(cfgDescription, null);
+		parser.processLine("command -Iargwithbackreferencearg file.cpp");
+		parser.processLine("command -Isimple file2.cpp");
+		parser.shutdown();
+
+		ICLanguageSetting ls = cfgDescription.getLanguageSettingForFile(file.getProjectRelativePath(), true);
+		String languageId = ls.getLanguageId();
+
+		List<ICLanguageSettingEntry> entries = parser.getSettingEntries(cfgDescription, file, languageId);
+		assertEquals(1, entries.size());
+		assertEquals(new CIncludePathEntry("/${ProjName}/backreferenceworks", ICSettingEntry.VALUE_WORKSPACE_PATH),
+				entries.get(0));
+
+		entries = parser.getSettingEntries(cfgDescription, file2, languageId);
+		assertEquals(1, entries.size());
+		assertEquals(new CIncludePathEntry("/${ProjName}/simple", ICSettingEntry.VALUE_WORKSPACE_PATH), entries.get(0));
+	}
+
 	/**
 	 * Test parsing of one typical entry.
 	 */

build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/AbstractLanguageSettingsOutputScanner.java

@@ -159,8 +159,10 @@ public abstract class AbstractLanguageSettingsOutputScanner extends LanguageSett
 		 *
 		 * @param kind - kind of language settings entries being parsed by the parser.
 		 * @param pattern - regular expression pattern being parsed by the parser.
-		 * @param nameExpression - capturing group expression defining name of an entry.
+		 * 					The pattern may be embedded into another pattern for intermediate
-		 * @param valueExpression - capturing group expression defining value of an entry.
+		 * 					parsing so it is best to avoid using numbered group back-reference e.g. \1
+		 * @param nameExpression - capturing group expression (numbered or named) defining name of an entry.
+		 * @param valueExpression - capturing group expression (numbered or named) defining value of an entry.
 		 * @param extraFlag - extra-flag to add while creating language settings entry.
 		 */
 		public AbstractOptionParser(int kind, String pattern, String nameExpression, String valueExpression,

build/org.eclipse.cdt.managedbuilder.core/src/org/eclipse/cdt/managedbuilder/language/settings/providers/GCCBuildCommandParser.java

@@ -32,24 +32,26 @@ import org.eclipse.cdt.core.settings.model.ICSettingEntry;
  */
 public class GCCBuildCommandParser extends AbstractBuildCommandParser implements ILanguageSettingsEditableProvider {
 	@SuppressWarnings("nls")
-	static final AbstractOptionParser[] optionParsers = { new IncludePathOptionParser("-I\\s*([\"'])(.*)\\1", "$2"),
+	static final AbstractOptionParser[] optionParsers = {
+			new IncludePathOptionParser("-I\\s*(?<quote>[\"'])(.*)\\k<quote>", "$2"),
 			new IncludePathOptionParser("-I\\s*([^\\s\"']*)", "$1"),
-			new IncludePathOptionParser("-isystem\\s*([\"'])(.*)\\1", "$2"),
+			new IncludePathOptionParser("-isystem\\s*(?<quote>[\"'])(.*)\\k<quote>", "$2"),
 			new IncludePathOptionParser("-isystem\\s*([^\\s\"']*)", "$1"),
-			new IncludePathOptionParser("-(F|(iframework))\\s*([\"'])(.*)\\3", "$4", ICSettingEntry.FRAMEWORKS_MAC),
+			new IncludePathOptionParser("-(F|(iframework))\\s*(?<quote>[\"'])(.*)\\k<quote>", "$4",
+					ICSettingEntry.FRAMEWORKS_MAC),
 			new IncludePathOptionParser("-(F|(iframework))\\s*([^\\s\"']*)", "$3", ICSettingEntry.FRAMEWORKS_MAC),
-			new IncludeFileOptionParser("-include\\s*([\"'])(.*)\\1", "$2"),
+			new IncludeFileOptionParser("-include\\s*(?<quote>[\"'])(.*)\\k<quote>", "$2"),
 			new IncludeFileOptionParser("-include\\s*([^\\s\"']*)", "$1"),
-			new MacroOptionParser("-D\\s*([\"'])([^=]*)(=(.*))?\\1", "$2", "$4"),
+			new MacroOptionParser("-D\\s*(?<quote>[\"'])([^=]*)(=(.*))?\\k<quote>", "$2", "$4"),
 			new MacroOptionParser("-D\\s*([^\\s=\"']*)=(\"\\\\(\")(.*?)\\\\\"\")", "$1", "$3$4$3"),
-			new MacroOptionParser("-D\\s*([^\\s=\"']*)=(\\\\([\"']))(.*?)\\2", "$1", "$3$4$3"),
+			new MacroOptionParser("-D\\s*([^\\s=\"']*)=(?<quote>\\\\([\"']))(.*?)\\k<quote>", "$1", "$3$4$3"),
-			new MacroOptionParser("-D\\s*([^\\s=\"']*)=([\"'])(.*?)\\2", "$1", "$3"),
+			new MacroOptionParser("-D\\s*([^\\s=\"']*)=(?<quote>[\"'])(.*?)\\k<quote>", "$1", "$3"),
 			new MacroOptionParser("-D\\s*([^\\s=\"']*)=([^\\s\"']*)?", "$1", "$2"),
 			new MacroOptionParser("-D\\s*([^\\s=\"']*)", "$1", "1"),
 			new MacroOptionParser("-U\\s*([^\\s=\"']*)", "$1", ICSettingEntry.UNDEFINED),
-			new MacroFileOptionParser("-imacros\\s*([\"'])(.*)\\1", "$2"),
+			new MacroFileOptionParser("-imacros\\s*(?<quote>[\"'])(.*)\\k<quote>", "$2"),
 			new MacroFileOptionParser("-imacros\\s*([^\\s\"']*)", "$1"),
-			new LibraryPathOptionParser("-L\\s*([\"'])(.*)\\1", "$2"),
+			new LibraryPathOptionParser("-L\\s*(?<quote>[\"'])(.*)\\k<quote>", "$2"),
 			new LibraryPathOptionParser("-L\\s*([^\\s\"']*)", "$1"),
 			new LibraryFileOptionParser("-l\\s*([^\\s\"']*)", "lib$1.a"), };