[packchk] Issue a warning for description attributes and elements that exceed the recommended limit of 128 (US ASCII) characters #1324 (#1141) (#1965)

* [packchk] Issue a warning for description attributes and elements that
exceed the recommended limit of 128 (US ASCII) characters #1324

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

Author: spcaipers-arm

tools/packchk/include/CheckFiles.h

@@ -31,6 +31,7 @@ class CheckFiles {
   const std::string& GetPackagePath() const;
   bool CheckFile(RteItem* item);
   bool CheckUrls(RteItem* item);
+  bool CheckDescription(RteItem* item);
   bool CheckFileExists(const std::string& fileName, int lineNo, bool associated = false);
   bool CheckCaseSense(const std::string& fileName, int lineNo);
   bool CheckFileIsInPack(const std::string& fileName, int lineNo);
@@ -47,7 +48,6 @@ 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 CheckPath(const std::string& fileName, int lineNo);
 
 private:
   std::string m_packagePath;

tools/packchk/src/CheckFiles.cpp

@@ -41,6 +41,7 @@ VISIT_RESULT CheckFilesVisitor::Visit(RteItem* item)
 {
   m_checkFiles.CheckFile(item);
   m_checkFiles.CheckUrls(item);
+  m_checkFiles.CheckDescription(item);
 
   return VISIT_RESULT::CONTINUE_VISIT;
 }
@@ -171,6 +172,38 @@ bool CheckFiles::ToUpper(string& text)
   return true;
 }
 
+/**
+ * @brief check aspects of an RTE url item
+ * @param item RteItem item to check
+ * @return passed / failed
+*/
+bool CheckFiles::CheckDescription(RteItem* item)
+{
+  if(!item) {
+    return true;
+  }
+
+  const auto& tag = item->GetTag();
+  const auto lineNo = item->GetLineNumber();
+  if(tag == "description") {
+    const auto& text = item->GetText();
+
+    for(auto c : text) {
+      if(c < 0x20 || c > 0x7e) {
+        LogMsg("M388", NAME(text), lineNo);
+        break;
+      }
+    }
+
+    if(text.length() > 128) {
+      LogMsg("M387", TAG(tag), lineNo);
+      return false;
+    }
+  }
+
+  return true;
+}
+
 /**
  * @brief check aspects of an RTE url item
  * @param item RteItem item to check

tools/packchk/src/PackChk_Msgs.cpp

@@ -203,6 +203,8 @@ const MsgTable PackChk::msgTable = {
   { "M384", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Condition '%NAME%': Direct or indirect recursion detected. Skipping condition while searching for '%NAME2%'." } },
   { "M385", { MsgLevel::LEVEL_INFO,     CRLF_B, "Release date is empty." } },
   { "M386", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Release date is in future: '%RELEASEDATE%' (today: %TODAYDATE%)" } },
+  { "M387", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Description in '%TAG%' is exceeding the recommended 128 (US ASCII) characters." } },
+  { "M388", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Unsupported character(s) found in '%NAME%'" } },
   { "M389", { MsgLevel::LEVEL_WARNING,  CRLF_B, "The component '%NAME%' (Line %LINE%) has dependency '%EXPR%' : '%NAME2%' that is resolved by the component itself." } },
   { "M390", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Condition '%NAME%': Direct or indirect recursion detected. %MSG%" } },
   { "M391", { MsgLevel::LEVEL_WARNING3, CRLF_B, "Redefined Item '%NAME%': %MSG%" } },

tools/packchk/test/data/TestTextExceedsLength/Files/header1.h

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

tools/packchk/test/data/TestTextExceedsLength/Files/test1.c

@@ -0,0 +1 @@
+// tests1.c

tools/packchk/test/data/TestTextExceedsLength/TestVendor.TestTextExceedsLength.pdsc

@@ -0,0 +1,50 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<package schemaVersion="1.4" xmlns:xs="http://www.w3.org/2001/XMLSchema-instance" xs:noNamespaceSchemaLocation="PACK.xsd">
+  <vendor>TestVendor</vendor>
+  <url>https://www.testurl.foo/pack/</url>
+  <name>TestTextExceedsLength</name>
+  <description>TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength TestTextExceedsLength </description>
+
+  <releases>
+    <release version="0.0.1" date="2022-06-20">
+      Initial release of TestTextExceedsLength.
+    </release>
+  </releases>
+
+  <keywords>
+    <keyword>TestTextExceedsLength</keyword>
+  </keywords>
+
+  <conditions>
+    <condition id="Test_Condition">
+      <description>Test Device</description>
+      <require Dvendor="ARM:82"/>
+    </condition>
+  </conditions>
+
+  <components>
+    <component Cclass="TestClass" Cgroup="TestGlobal" Cversion="1.0.0" condition="Test_Condition">
+      <description>Unsupported char € </description>
+    </component>
+    <component Cclass="Device" Cgroup="Config Tools" Csub="Init" Cversion="1.0.0" condition="Test_Condition">
+      <description>Initialization generated by MyConfig</description>
+      <files>
+        <file category="source" name="Files/test1.c"/>
+      </files>
+    </component>
+  </components>
+  
+  <devices>
+    <family Dfamily="MyDevice Series" Dvendor="ARM:82">
+      <processor Dcore="Cortex-M3" DcoreVersion="r1p1"/>
+
+      <subFamily DsubFamily="MyDevice">
+        <device Dname="MyDeviceM0">
+          <processor Dfpu="0" Dmpu="0" Dendian="Little-endian" Dclock="24000000"/>
+        </device>
+      </subFamily>
+    </family>
+  </devices>
+
+</package>

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

@@ -823,6 +823,7 @@ TEST_F(PackChkIntegTests, CheckDescriptionOverviewMd) {
     FAIL() << "error: message M337 found";
   }
 }
+
 // Validate config file vs. include path
 TEST_F(PackChkIntegTests, CheckConfigFileInIncludePath) {
   const char* argv[3];
@@ -852,6 +853,43 @@ TEST_F(PackChkIntegTests, CheckConfigFileInIncludePath) {
   }
 }
 
+// Validate config file vs. include path
+TEST_F(PackChkIntegTests, CheckTestTextExceedsLength) {
+  const char* argv[7];
+
+  const string& pdscFile = PackChkIntegTestEnv::localtestdata_dir +
+    "/TestTextExceedsLength/TestVendor.TestTextExceedsLength.pdsc";
+  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
+
+  argv[0] = (char*)"";
+  argv[1] = (char*)pdscFile.c_str();
+  argv[2] = (char *)"--disable-validation";
+  argv[3] = (char*)"-x";
+  argv[4] = (char*)"!M387";
+  argv[5] = (char*)"-x";
+  argv[6] = (char*)"!M388";
+
+  PackChk packChk;
+  EXPECT_EQ(0, packChk.Check(7, argv, nullptr));
+
+  auto errMsgs = ErrLog::Get()->GetLogMessages();
+  int M387_foundCnt = 0;
+  int M388_foundCnt = 0;
+  for (const string& msg : errMsgs) {
+    size_t s;
+    if ((s = msg.find("M387")) != string::npos) {
+      M387_foundCnt++;
+    }
+    if ((s = msg.find("M388")) != string::npos) {
+      M388_foundCnt++;
+    }
+  }
+
+  if (!M387_foundCnt || !M388_foundCnt) {
+    FAIL() << "error: missing message M387 or M388";
+  }
+}
+
 // Test schema validation
 TEST_F(PackChkIntegTests, CheckSchemaValidation) {
   const char* argv[3];