[treeplayer] Don't mistake formulas for script files in TTree::Draw

tree/treeplayer/src/TTreePlayer.cxx

@@ -31,6 +31,7 @@ extra libraries (Histogram, display, etc).
 #include <cstdlib>
 #include <iostream>
 #include <fstream>
+#include <vector>
 
 #include "TROOT.h"
 #include "TApplication.h"
@@ -274,6 +275,48 @@ void TTreePlayer::DeleteSelectorFromFile()
    fSelectorClass = nullptr;
 }
 
+namespace {
+
+// TTree::Draw can take the name of a C++ script file (optionally with an
+// ACLiC mode suffix and arguments, e.g. "myscript.C+(2)") instead of a
+// TTreeFormula expression for both the variable expression and the
+// selection; see "Drawing a user function accessing the TTree data directly"
+// in the TTree::Draw documentation. Determine whether 'expression' names
+// such a script file.
+//
+// Besides checking that the file exists, we require that the candidate has an
+// extension: TTreeProxyGenerator::WriteProxy() needs one to derive the name of
+// the function to call. This avoids misinterpreting an expression like "abs(x)"
+// as the script "abs" called with argument "(x)" whenever an unrelated file
+// with that name happens to exist in the current directory (JIRA ROOT-8000).
+
+bool IsScriptFile(const char *expression)
+{
+   if (!expression || !expression[0])
+      return false;
+
+   const TString candidate = expression;
+
+   // These tokens only appear in TTreeFormula expressions, never in the name of
+   // a script file, so their presence rules out a script file.
+   static const std::vector<TString> formulaTokens = {
+      "Alt$", "Entries$", "LocalEntries$", "Length$",    "Entry$", "LocalEntry$", "Min$",
+      "Max$", "MinIf$",   "MaxIf$",        "Iteration$", "Sum$",   ">",           "<"};
+   for (const TString &token : formulaTokens)
+      if (candidate.Index(token) >= 0)
+         return false;
+
+   TString aclicMode, arguments, io;
+   const TString realname = gSystem->SplitAclicMode(candidate, aclicMode, arguments, io);
+   const Ssiz_t dot_pos = realname.Last('.');
+   if (dot_pos == kNPOS || dot_pos < realname.Last('/'))
+      return false;
+
+   return gSystem->IsFileInIncludePath(candidate);
+}
+
+} // anonymous namespace
+
 ////////////////////////////////////////////////////////////////////////////////
 /// Draw the result of a C++ script.
 ///
@@ -357,44 +400,24 @@ Long64_t TTreePlayer::DrawSelect(const char *varexp0, const char *selection, Opt
    // Let's see if we have a filename as arguments instead of
    // a TTreeFormula expression.
 
-   TString possibleFilename = varexp0;
-   Ssiz_t dot_pos = possibleFilename.Last('.');
-   if ( dot_pos != kNPOS
-       && possibleFilename.Index("Alt$")<0 && possibleFilename.Index("Entries$")<0
-       && possibleFilename.Index("LocalEntries$")<0
-       && possibleFilename.Index("Length$")<0  && possibleFilename.Index("Entry$")<0
-       && possibleFilename.Index("LocalEntry$")<0
-       && possibleFilename.Index("Min$")<0 && possibleFilename.Index("Max$")<0
-       && possibleFilename.Index("MinIf$")<0 && possibleFilename.Index("MaxIf$")<0
-       && possibleFilename.Index("Iteration$")<0 && possibleFilename.Index("Sum$")<0
-       && possibleFilename.Index(">")<0 && possibleFilename.Index("<")<0
-       && gSystem->IsFileInIncludePath(possibleFilename.Data())) {
-
-      if (selection && strlen(selection) && !gSystem->IsFileInIncludePath(selection)) {
+   if (IsScriptFile(varexp0)) {
+
+      if (selection && strlen(selection) && !IsScriptFile(selection)) {
          Error("DrawSelect",
-               "Drawing using a C++ file currently requires that both the expression and the selection are files\n\t\"%s\" is not a file",
+               "Drawing using a C++ macro currently requires that both the expression and the selection are "
+               "files\n\t\"%s\" is not a file",
                selection);
          return 0;
       }
       return DrawScript("generatedSel",varexp0,selection,option,nentries,firstentry);
 
-   } else {
-      possibleFilename = selection;
-      if (possibleFilename.Index("Alt$")<0 && possibleFilename.Index("Entries$")<0
-          && possibleFilename.Index("LocalEntries$")<0
-          && possibleFilename.Index("Length$")<0  && possibleFilename.Index("Entry$")<0
-          && possibleFilename.Index("LocalEntry$")<0
-          && possibleFilename.Index("Min$")<0 && possibleFilename.Index("Max$")<0
-          && possibleFilename.Index("MinIf$")<0 && possibleFilename.Index("MaxIf$")<0
-          && possibleFilename.Index("Iteration$")<0 && possibleFilename.Index("Sum$")<0
-          && possibleFilename.Index(">")<0 && possibleFilename.Index("<")<0
-          && gSystem->IsFileInIncludePath(possibleFilename.Data())) {
+   } else if (IsScriptFile(selection)) {
 
-         Error("DrawSelect",
-               "Drawing using a C++ file currently requires that both the expression and the selection are files\n\t\"%s\" is not a file",
-               varexp0);
-         return 0;
-      }
+      Error("DrawSelect",
+            "Drawing using a C++ macro currently requires that both the expression and the selection are "
+            "files\n\t\"%s\" is not a file",
+            varexp0);
+      return 0;
    }
 
    Long64_t oldEstimate  = fTree->GetEstimate();

tree/treeplayer/test/regressions.cxx

@@ -359,6 +359,71 @@ TEST(TTreeReaderRegressions, ValueFastTuple)
    EXPECT_NEAR(total, 866026269930.1345215, 100);
 }
 
+// ROOT-8000 https://its.cern.ch/jira/browse/ROOT-8000
+// An unrelated file lying around in the current directory whose name matches
+// the leading part of a formula (e.g. a file named "abs") must not make
+// TTree::Draw interpret the formula as the name of a C++ script called with
+// arguments: only files with an extension qualify as scripts.
+TEST(TTreeDrawRegressions, ExpressionNotShadowedByFile)
+{
+   ROOT::TestSupport::FileRaii shadowingFile{"abs"};
+   {
+      std::ofstream f(shadowingFile.GetPath());
+      f << "This is a text file, not a C++ script.\n";
+   }
+
+   TTree t("t", "t");
+   double x;
+   int id;
+   t.Branch("x", &x);
+   t.Branch("id", &id);
+   for (int i = 0; i < 100; ++i) {
+      x = i - 50;
+      id = (i % 2) ? 531 : -531;
+      t.Fill();
+   }
+
+   // These formulas end with a parenthesized part, which used to be stripped
+   // as ACLiC-style arguments, leaving "abs" to be found as a file.
+   EXPECT_EQ(t.Draw("abs(x)", "", "goff"), 100);
+   EXPECT_EQ(t.Draw("x", "abs(id)", "goff"), 100);
+   // The dot in "0.5" used to make the varexp pass the extension check.
+   EXPECT_EQ(t.Draw("abs(x + 0.5)", "abs(id) == 531", "goff"), 100);
+   EXPECT_EQ(t.Draw("x", "abs(id) == id", "goff"), 50);
+}
+
+// ROOT-8000 https://its.cern.ch/jira/browse/ROOT-8000
+// The flip side of the test above: a *real* macro whose name matches the
+// leading part of a formula must still be recognized as a script, precisely
+// because it has an extension. We check this without paying for the ACLiC
+// compilation of the macro: when the variable expression is a script file but
+// the selection is a plain formula (not a file), DrawSelect must reject the
+// combination with its "both must be files" error. That error is only reached
+// if "abs.C" was taken as a script - a bare "abs" would be treated as a formula
+// and the error above ("... is not a file" for the selection) would never fire.
+TEST(TTreeDrawRegressions, MacroWithExtensionRecognizedAsScript)
+{
+   ROOT::TestSupport::FileRaii macroFile{"abs.C"};
+   {
+      std::ofstream f(macroFile.GetPath());
+      f << "double abs()\n{\n   return x;\n}\n";
+   }
+
+   TTree t("t", "t");
+   double x;
+   t.Branch("x", &x);
+   for (int i = 0; i < 100; ++i) {
+      x = i - 50;
+      t.Fill();
+   }
+
+   ROOT::TestSupport::CheckDiagsRAII diags{kError, "TTreePlayer::DrawSelect",
+                                           "Drawing using a C++ macro currently requires that both the expression and "
+                                           "the selection are files",
+                                           /*matchFullMessage=*/false};
+   EXPECT_EQ(t.Draw("abs.C", "x > 0", "goff"), 0);
+}
+
 // https://github.com/root-project/root/issues/20226
 TEST(TTreeScan, IntOverflow)
 {