Merge topic 'fix-CMP0054-elseif-warning'

d6a03b47 cmIfCommand: Issue CMP0054 warning with appropriate context. (#15802)

10 files changed, 108 insertions, 12 deletions

diff --git a/Source/cmConditionEvaluator.cxx b/Source/cmConditionEvaluator.cxx
index 7874803..5330acd 100644
--- a/Source/cmConditionEvaluator.cxx
+++ b/Source/cmConditionEvaluator.cxx
@@ -11,9 +11,14 @@
 ============================================================================*/
 
 #include "cmConditionEvaluator.h"
+#include "cmOutputConverter.h"
 
-cmConditionEvaluator::cmConditionEvaluator(cmMakefile& makefile):
+cmConditionEvaluator::cmConditionEvaluator(cmMakefile& makefile,
+                                           const cmListFileContext &context,
+                                           const cmListFileBacktrace& bt):
   Makefile(makefile),
+  ExecutionContext(context),
+  Backtrace(bt),
   Policy12Status(makefile.GetPolicyStatus(cmPolicies::CMP0012)),
   Policy54Status(makefile.GetPolicyStatus(cmPolicies::CMP0054)),
   Policy57Status(makefile.GetPolicyStatus(cmPolicies::CMP0057)),
@@ -98,6 +103,25 @@ bool cmConditionEvaluator::IsTrue(
     errorString, status, true);
 }
 
+cmListFileContext cmConditionEvaluator::GetConditionContext(
+    cmMakefile* mf,
+    const cmCommandContext& command,
+    const std::string& filePath)
+{
+  cmListFileContext context =
+      cmListFileContext::FromCommandContext(
+        command,
+        filePath);
+
+  if(!mf->GetCMakeInstance()->GetIsInTryCompile())
+    {
+    cmOutputConverter converter(mf->GetStateSnapshot());
+    context.FilePath = converter.Convert(context.FilePath,
+                                         cmOutputConverter::HOME);
+    }
+  return context;
+}
+
 //=========================================================================
 const char* cmConditionEvaluator::GetDefinitionIfUnquoted(
   cmExpandedCommandArgument const& argument) const
@@ -113,7 +137,8 @@ const char* cmConditionEvaluator::GetDefinitionIfUnquoted(
 
   if(def && argument.WasQuoted() && this->Policy54Status == cmPolicies::WARN)
     {
-    if(!this->Makefile.HasCMP0054AlreadyBeenReported())
+    if(!this->Makefile.HasCMP0054AlreadyBeenReported(
+         this->ExecutionContext))
       {
       std::ostringstream e;
       e << (cmPolicies::GetPolicyWarning(cmPolicies::CMP0054)) << "\n";
@@ -122,7 +147,9 @@ const char* cmConditionEvaluator::GetDefinitionIfUnquoted(
         "when the policy is set to NEW.  "
         "Since the policy is not set the OLD behavior will be used.";
 
-      this->Makefile.IssueMessage(cmake::AUTHOR_WARNING, e.str());
+      this->Makefile.GetCMakeInstance()
+          ->IssueMessage(cmake::AUTHOR_WARNING, e.str(),
+                         this->Backtrace);
       }
     }
 
@@ -159,7 +186,8 @@ bool cmConditionEvaluator::IsKeyword(std::string const& keyword,
   if(isKeyword && argument.WasQuoted() &&
     this->Policy54Status == cmPolicies::WARN)
     {
-    if(!this->Makefile.HasCMP0054AlreadyBeenReported())
+    if(!this->Makefile.HasCMP0054AlreadyBeenReported(
+         this->ExecutionContext))
       {
       std::ostringstream e;
       e << cmPolicies::GetPolicyWarning(cmPolicies::CMP0054) << "\n";
@@ -168,7 +196,9 @@ bool cmConditionEvaluator::IsKeyword(std::string const& keyword,
         "when the policy is set to NEW.  "
         "Since the policy is not set the OLD behavior will be used.";
 
-      this->Makefile.IssueMessage(cmake::AUTHOR_WARNING, e.str());
+      this->Makefile.GetCMakeInstance()
+          ->IssueMessage(cmake::AUTHOR_WARNING, e.str(),
+                         this->Backtrace);
       }
     }
 
diff --git a/Source/cmConditionEvaluator.h b/Source/cmConditionEvaluator.h
index c4e2d11..8600825 100644
--- a/Source/cmConditionEvaluator.h
+++ b/Source/cmConditionEvaluator.h
@@ -22,7 +22,9 @@ class cmConditionEvaluator
 public:
   typedef std::list<cmExpandedCommandArgument> cmArgumentList;
 
-  cmConditionEvaluator(cmMakefile& makefile);
+  cmConditionEvaluator(cmMakefile& makefile,
+                       cmListFileContext const& context,
+                       cmListFileBacktrace const& bt);
 
   // this is a shared function for both If and Else to determine if the
   // arguments were valid, and if so, was the response true. If there is
@@ -31,6 +33,9 @@ public:
       std::string &errorString,
       cmake::MessageType &status);
 
+  static cmListFileContext GetConditionContext(cmMakefile* mf,
+      const cmCommandContext& command, std::string const& filePath);
+
 private:
   // Filter the given variable definition based on policy CMP0054.
   const char* GetDefinitionIfUnquoted(
@@ -91,6 +96,8 @@ private:
       cmake::MessageType &status);
 
   cmMakefile& Makefile;
+  cmListFileContext ExecutionContext;
+  cmListFileBacktrace Backtrace;
   cmPolicies::PolicyStatus Policy12Status;
   cmPolicies::PolicyStatus Policy54Status;
   cmPolicies::PolicyStatus Policy57Status;
diff --git a/Source/cmIfCommand.cxx b/Source/cmIfCommand.cxx
index a03808f..5964ef1 100644
--- a/Source/cmIfCommand.cxx
+++ b/Source/cmIfCommand.cxx
@@ -107,7 +107,14 @@ IsFunctionBlocked(const cmListFileFunction& lff,
 
             cmake::MessageType messType;
 
-            cmConditionEvaluator conditionEvaluator(mf);
+            cmListFileContext conditionContext =
+                cmConditionEvaluator::GetConditionContext(
+                  &mf, this->Functions[c],
+                  this->GetStartingContext().FilePath);
+
+            cmConditionEvaluator conditionEvaluator(
+                  mf, conditionContext,
+                  mf.GetBacktrace(this->Functions[c]));
 
             bool isTrue = conditionEvaluator.IsTrue(
               expandedArguments, errorString, messType);
@@ -196,7 +203,16 @@ bool cmIfCommand
 
   cmake::MessageType status;
 
-  cmConditionEvaluator conditionEvaluator(*(this->Makefile));
+  cmListFileContext execContext = this->Makefile->GetExecutionContext();
+
+  cmCommandContext commandContext;
+  commandContext.Line = execContext.Line;
+  commandContext.Name = execContext.Name;
+
+  cmConditionEvaluator conditionEvaluator(
+        *(this->Makefile), cmConditionEvaluator::GetConditionContext(
+          this->Makefile, commandContext, execContext.FilePath),
+        this->Makefile->GetBacktrace());
 
   bool isTrue = conditionEvaluator.IsTrue(
     expandedArguments, errorString, status);
diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx
index a1f143a..264b169 100644
--- a/Source/cmMakefile.cxx
+++ b/Source/cmMakefile.cxx
@@ -4569,9 +4569,10 @@ bool cmMakefile::SetPolicyVersion(const char *version)
 }
 
 //----------------------------------------------------------------------------
-bool cmMakefile::HasCMP0054AlreadyBeenReported() const
+bool cmMakefile::HasCMP0054AlreadyBeenReported(
+    cmListFileContext const& context) const
 {
-  return !this->CMP0054ReportedIds.insert(this->GetExecutionContext()).second;
+  return !this->CMP0054ReportedIds.insert(context).second;
 }
 
 //----------------------------------------------------------------------------
diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h
index 4215b72..0b8db3c 100644
--- a/Source/cmMakefile.h
+++ b/Source/cmMakefile.h
@@ -331,7 +331,7 @@ public:
    * Determine if the given context, name pair has already been reported
    * in context of CMP0054.
    */
-  bool HasCMP0054AlreadyBeenReported() const;
+  bool HasCMP0054AlreadyBeenReported(const cmListFileContext &context) const;
 
   bool IgnoreErrorsCMP0061() const;
 
diff --git a/Source/cmWhileCommand.cxx b/Source/cmWhileCommand.cxx
index 012c580..4b7afd8 100644
--- a/Source/cmWhileCommand.cxx
+++ b/Source/cmWhileCommand.cxx
@@ -49,7 +49,20 @@ IsFunctionBlocked(const cmListFileFunction& lff, cmMakefile &mf,
       mf.ExpandArguments(this->Args, expandedArguments);
       cmake::MessageType messageType;
 
-      cmConditionEvaluator conditionEvaluator(mf);
+      cmListFileContext execContext = this->GetStartingContext();
+
+      cmCommandContext commandContext;
+      commandContext.Line = execContext.Line;
+      commandContext.Name = execContext.Name;
+
+      cmListFileContext conditionContext =
+          cmConditionEvaluator::GetConditionContext(
+            &mf, commandContext,
+            this->GetStartingContext().FilePath);
+
+      cmConditionEvaluator conditionEvaluator(
+            mf, conditionContext,
+            mf.GetBacktrace(commandContext));
 
       bool isTrue = conditionEvaluator.IsTrue(
         expandedArguments, errorString, messageType);
diff --git a/Tests/RunCMake/CMP0054/CMP0054-WARN-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-WARN-stderr.txt
index 3d875ae..3cfa5d2 100644
--- a/Tests/RunCMake/CMP0054/CMP0054-WARN-stderr.txt
+++ b/Tests/RunCMake/CMP0054/CMP0054-WARN-stderr.txt
@@ -9,3 +9,15 @@ CMake Warning \(dev\) at CMP0054-WARN.cmake:3 \(if\):
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 This warning is for project developers.  Use -Wno-dev to suppress it.
++
+CMake Warning \(dev\) at CMP0054-WARN.cmake:5 \(elseif\):
+  Policy CMP0054 is not set: Only interpret if\(\) arguments as variables or
+  keywords when unquoted.  Run "cmake --help-policy CMP0054" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
+
+  Quoted variables like "FOO" will no longer be dereferenced when the policy
+  is set to NEW.  Since the policy is not set the OLD behavior will be used.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
diff --git a/Tests/RunCMake/CMP0054/CMP0054-WARN.cmake b/Tests/RunCMake/CMP0054/CMP0054-WARN.cmake
index 37855fc..a608929 100644
--- a/Tests/RunCMake/CMP0054/CMP0054-WARN.cmake
+++ b/Tests/RunCMake/CMP0054/CMP0054-WARN.cmake
@@ -2,4 +2,6 @@ set(FOO "BAR")
 
 if(NOT "FOO" STREQUAL "BAR")
   message(FATAL_ERROR "The given literals should match")
+elseif("FOO" STREQUAL "BING")
+  message(FATAL_ERROR "The given literals should not match")
 endif()
diff --git a/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN-stderr.txt b/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN-stderr.txt
index b1ebd49..5a8c263 100644
--- a/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN-stderr.txt
+++ b/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN-stderr.txt
@@ -10,3 +10,16 @@ CMake Warning \(dev\) at CMP0054-keywords-WARN.cmake:1 \(if\):
 Call Stack \(most recent call first\):
   CMakeLists.txt:3 \(include\)
 This warning is for project developers.  Use -Wno-dev to suppress it.
++
+CMake Warning \(dev\) at CMP0054-keywords-WARN.cmake:3 \(elseif\):
+  Policy CMP0054 is not set: Only interpret if\(\) arguments as variables or
+  keywords when unquoted.  Run "cmake --help-policy CMP0054" for policy
+  details.  Use the cmake_policy command to set the policy and suppress this
+  warning.
+
+  Quoted keywords like "DEFINED" will no longer be interpreted as keywords
+  when the policy is set to NEW.  Since the policy is not set the OLD
+  behavior will be used.
+Call Stack \(most recent call first\):
+  CMakeLists.txt:3 \(include\)
+This warning is for project developers.  Use -Wno-dev to suppress it.
diff --git a/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN.cmake b/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN.cmake
index ee0a623..118ab3d 100644
--- a/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN.cmake
+++ b/Tests/RunCMake/CMP0054/CMP0054-keywords-WARN.cmake
@@ -1,3 +1,5 @@
 if("NOT" 1)
   message(FATAL_ERROR "[\"NOT\" 1] evaluated true")
+elseif("DEFINED" NotDefined)
+  message(FATAL_ERROR "[\"DEFINED\" NotDefined] evaluated true")
 endif()