[packchk] issue warning for explicit include paths in *.pdsc to config files #1644 (#1999)

* [packchk] issue warning for explicit include paths in *.pdsc to config
files #1644

* fixed test

Co-authored-by: Thorsten de Buhr <[email protected]>

Author: grasci-arm

tools/packchk/include/CheckFiles.h

@@ -49,10 +49,14 @@ class CheckFiles {
   bool GetFileName(RteItem* item, std::string& filename, FileType& fileType) const;
   bool ToUpper(std::string& text);
   std::string GetFullFilename(const std::string& fileName);
+  bool GatherIncPathVsAttrConfig(RteItem* item);
+  bool CheckAttrConfigFiles();
 
 private:
   std::string m_packagePath;
   std::string m_packageName;
+  std::map<std::string, RteItem*> m_includePaths;
+  std::map<std::string, RteItem*> m_attrConfigFiles;
 };
 
 class CheckFilesVisitor : public RteVisitor

tools/packchk/src/CheckFiles.cpp

@@ -30,6 +30,7 @@ CheckFilesVisitor::CheckFilesVisitor(const string& packagePath, const string& pa
 */
 CheckFilesVisitor::~CheckFilesVisitor()
 {
+  m_checkFiles.CheckAttrConfigFiles();
 }
 
 /**
@@ -43,6 +44,7 @@ VISIT_RESULT CheckFilesVisitor::Visit(RteItem* item)
   m_checkFiles.CheckUrls(item);
   m_checkFiles.CheckDeprecated(item);
   m_checkFiles.CheckDescription(item);
+  m_checkFiles.GatherIncPathVsAttrConfig(item);
 
   return VISIT_RESULT::CONTINUE_VISIT;
 }
@@ -896,3 +898,57 @@ bool CheckFiles::CheckFileExtension(RteItem* item)
 
   return ok;
 }
+
+
+bool CheckFiles::GatherIncPathVsAttrConfig(RteItem* item)
+{
+  const auto file = dynamic_cast<RteFile*>(item);
+  if(!file) {
+    return true;
+  }
+
+  if(file->IsTemplate()) {    // ignore
+    return true;
+  }
+
+  const auto category = file->GetCategory();
+
+  if(file->IsConfig()) {
+    const auto path = file->GetOriginalAbsolutePath();
+    m_attrConfigFiles[path] = file;
+    return true;
+  }
+
+  if(category == RteFile::Category::HEADER) {
+    const auto incPath = file->GetIncludePath();
+    m_includePaths[incPath] = file;
+    return true;
+  }
+
+  if(category == RteFile::Category::INCLUDE) {
+    const auto incPath = file->GetOriginalAbsolutePath();
+    m_includePaths[incPath] = file;
+    return true;
+  }
+
+
+  return true;
+}
+
+bool CheckFiles::CheckAttrConfigFiles()
+{
+  bool bOk = true;
+
+  for(const auto& [path, item] : m_attrConfigFiles) {
+      const auto lineNo = item->GetLineNumber();
+      const auto incPathFound = m_includePaths.find(RteUtils::ExtractFilePath(path, false));
+      if(incPathFound != m_includePaths.end()) {
+        const auto itemFound = incPathFound->second;
+        LogMsg("M357", NAME(item->GetName()), LINE(itemFound->GetLineNumber()), lineNo);
+        bOk = false;
+      }
+  }
+
+  return bOk;
+}
+

tools/packchk/src/PackChk_Msgs.cpp

@@ -172,7 +172,7 @@ const MsgTable PackChk::msgTable = {
   { "M354", { MsgLevel::LEVEL_WARNING3, CRLF_B, "Multiple '%FILECAT%' Files found for Component '%COMP%' for '[%VENDOR%] %MCU%' (Compiler: %COMPILER% [%OPTION%])" } },
   { "M355", { MsgLevel::LEVEL_WARNING3, CRLF_B, "No '%FILECAT%' Directory found for Component '%COMP%' (%COMPID%) for '[%VENDOR%] %MCU%' (Compiler: %COMPILER% [%OPTION%])" } },
   { "M356", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Path '%PATH%' does not specify a file!" } },
-  { "M357", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Config File '%NAME%', which gets copied to project, is also found in the include path to the original pack folder. Do not place config header files in the same directory as regular header files." } },
+  { "M357", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Config File '%NAME%', which gets copied to project, is also found in the include path to the original pack folder. Do not place config header files in the same directory as regular header files. See Line %LINE%" } },
   { "M358", { MsgLevel::LEVEL_WARNING3, CRLF_B, "Header File '%HFILE%' for '%CFILE%', used by Component '%COMP%' (%COMPID%)\n"\
                                                 "  for Device '[%VENDOR%] %MCU%' (Compiler: %COMPILER% [%OPTION%]) not found on file system in any include paths: %PATH%" } },
   { "M359", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Family has no Device(s) or Subfamilies: '%FAMILY%" } },

tools/packchk/test/data/ConfigFileInIncludePath/Header/header3.h

@@ -0,0 +1 @@
+//header 1

tools/packchk/test/data/ConfigFileInIncludePath/TestVendor.ConfigFileInIncludePathPack.pdsc

@@ -58,11 +58,18 @@ The TestSubFamily is ...
       <description>System Startup for STMicroelectronics STM32F1xx device series</description>
       <files>
         <file category="include" name="Files/"/>
-        <file category="source"  name="Files/header1.h" attr="config" version="1.0.0" condition="Test ARMCC"/>
-        <file category="header"  name="Header/header1.h" version="1.0.0" condition="Test ARMCC"/>
+        <file category="source"  name="Files/header1.h"  attr="config" version="1.0.0" condition="Test ARMCC"/>
+        <file category="header"  name="Header/header1.h"               version="1.0.0" condition="Test ARMCC"/>
         <file category="header"  name="Header/header2.h" attr="config" version="1.0.0" condition="Test ARMCC"/>
       </files>
     </component>
+
+    <component Cclass="Device" Cgroup="TestComponent" Cversion="1.0.0" condition="TestDevices">
+      <description>System Startup for STMicroelectronics STM32F1xx device series</description>
+      <files>
+        <file category="header"  name="Header/header3.h" attr="config" version="1.0.0" condition="Test ARMCC"/>
+      </files>
+    </component>
   </components>
 
 </package>

tools/packchk/test/integtests/src/PackChkIntegTests.cpp

@@ -863,7 +863,7 @@ TEST_F(PackChkIntegTests, CheckConfigFileInIncludePath) {
     }
   }
 
-  if (M357_foundCnt != 2) {
+  if (M357_foundCnt != 5) {
     FAIL() << "error: missing message M357";
   }
 }