Merge pull request #37 from mPokornyETM/fixes-and-improvements

Fixes and improvements

.gitignore

@@ -0,0 +1,2 @@
+WinCCOA_QualityChecks/bin/ctrlppcheck/
+ctrlppcheck/build/

README.md

@@ -6,7 +6,7 @@ Introduction
 ------------
 
 WinCC OA CtrlppCheck is a set of tools for static code analysis for SIMATIC WinCC OA's scripting language Ctrl and Ctrl++.  
-It supports all versions of WinCC OA and is optimised for versions 3.19 and higher.
+It supports all versions of WinCC OA and is optimized for versions 3.19 and higher.
 They are easy to use and integrate perfectly with WinCC OA's script editor GEDI but may also be used in an CI toolchain for automated pull request builds etc.
 
 Community Resources

WinCCOA_QualityChecks/data/ctrlPpCheck/rule/ctrl_rules.xml → WinCCOA_QualityChecks/data/ctrlPpCheck/rule/ctrl.xml

@@ -98,15 +98,15 @@
     <message>
         <id>StrlenNotEmptyString</id>
         <severity>performance</severity>
-        <summary>Using strlen() to check if a string is not empty, is not efficient.</summary>
+        <summary>Using strlen() to check if a string is not empty, is not efficient. Use string::isEmpty() instead.</summary>
     </message>
 </rule>
 <rule version="1">
     <pattern>  strlen \( \S+ \) == 0 </pattern>
     <message>
         <id>StrlenEmptyString</id>
         <severity>performance</severity>
-        <summary>Using strlen() to check if a string is empty, is not efficient.</summary>
+        <summary>Using strlen() to check if a string is empty, is not efficient. Use string::isEmpty() instead.</summary>
     </message>
 </rule>
 <rule version="1">
@@ -115,7 +115,7 @@
     <message>
         <id>strPosEq0</id>
         <severity>performance</severity>
-        <summary>Using strpos(s) == 0 to check if the string start with other string is not efficient.</summary>
+        <summary>Using strpos(s) == 0 to check if the string start with other string is not efficient. Use string::startsWith() instead.</summary>
     </message>
 </rule>
 <!-- END OF: PERFORMANCE -->
@@ -149,3 +149,72 @@
 	</message>
 </rule>
 <!-- END OF: DEBUG -->
+<rule version="1">
+	<tokenlist>normal</tokenlist>
+	<pattern>system \( \S+ </pattern>
+	<message>
+	    <id>quoted_params_system</id>
+		<severity>error</severity>
+		<summary>Call function system() with unquoted parameters.
+		Calling function system() with unquoted parameters leads to issues, when spaces in parameter are used.
+		For example, when the program is located in path containing spaces.
+		Verify, that the system call has quoted parameters and suppres this error by
+		//ctrlppcheck-suppress quoted_params_system</summary>
+	</message>
+</rule>
+<rule version="1">
+	<tokenlist>normal</tokenlist>
+	<pattern>system \( \S+ </pattern>
+	<message>
+	    <id>unsafe_system_call</id>
+		<severity>warning</severity>
+		<summary>Calling system command outside WinCC OA.
+		Calling system command outside WinCC OA may be dangerours.
+		This is potential security issue.
+		Verify that this operation is safe and suppres this warning by
+		//ctrlppcheck-suppress unsafe_system_call</summary>
+	</message>
+</rule>
+<rule version="1">
+	<tokenlist>raw</tokenlist>
+	<pattern>makeError \( \S+ , \S+ , \S+ , 0 </pattern>
+	<message>
+	    <id>fatal_program_exit</id>
+		<severity>warning</severity>
+		<summary>Throw error with FATAL priority stop the program.
+		Throwing a error with FATAL priority leads to stop the program.
+		Verify that this scenario is accepted and suppres this warning by
+		//ctrlppcheck-suppress fatal_program_exit</summary>
+	</message>
+</rule>
+
+<rule version="1">
+	<tokenlist>raw</tokenlist>
+	<pattern>logger . fatal \( </pattern>
+	<message>
+	    <id>fatal_program_exit</id>
+		<severity>warning</severity>
+		<summary>Throw error with FATAL priority stop the program.
+		Throwing a error with FATAL priority leads to stop the program.
+		Verify that this scenario is accepted and suppres this warning by
+		//ctrlppcheck-suppress fatal_program_exit</summary>
+	</message>
+</rule>
+<rule version="1">
+	<tokenlist>normal</tokenlist>
+	<pattern>uses \".*\.ctl\"</pattern>
+	<message>
+	    <id>library_extension</id>
+		<severity>style</severity>
+		<summary>It is not neccessary to use .ctl extension in #uses.</summary>
+	</message>
+</rule>
+<rule version="1">
+	<tokenlist>normal</tokenlist>
+	<pattern>uses \".*\.ctc\"</pattern>
+	<message>
+	    <id>library_extension</id>
+		<severity>style</severity>
+		<summary>It is not neccessary to use .ctc extension in #uses.</summary>
+	</message>
+</rule>

WinCCOA_QualityChecks/data/ctrlPpCheck/rule/variableNaming.xml

@@ -7,14 +7,6 @@
  simple
 -->
 
-<rule version="1">
-	<pattern>^[A-Z0-9\\-\\_]+$</pattern>
-	<message>
-		<id>const</id>
-		<summary>The variable name cannot consist of lowercase letters.</summary>
-	</message>
-</rule>
-
 <rule version="1">
 	<pattern>^.+$</pattern>
 	<message>

WinCCOA_QualityChecks/msg/de_AT.utf8/QgBase.cat

@@ -1,3 +1,8 @@
-00001,QualityGates does not run succesfully !!!
+00001,QualityGates does not run successfully !!!
 00010,Assertion return error: $1
 00011,Assertion works: $1
+00020,The function $1 is not implemented.
+00021,Start quality gate $1.
+00022,Calculate sources for quality gate $1.
+00023,Validate results for quality gate $1.
+00024,Quality gate $1 done. It takes to mee $2 [s.ms].

WinCCOA_QualityChecks/msg/de_AT.utf8/QgStaticCheck_StaticDir.cat

@@ -1,7 +1,7 @@
 assert.dir.isCalculated,Is calculated
 reason.dir.isCalculated,Sorry, the directory '($dir.name)' can't be calculated.
 assert.dir.hasFilesRecursive,Count of files recursive
-reason.dir.hasFilesRecursive,Directory  '$dir.name' or sub directories doesn´t contain any files. Delete all empty dirs.
+reason.dir.hasFilesRecursive,Directory '$dir.name' or sub directories doesn´t contain any files. Delete all empty dirs.
 assert.dir.isEmpty,Is empty
 reason.dir.isEmpty,Directory '$dir.name' doesn´t contains any files or sub directories.
 assert.dir.subDirCount,Count of sub directories

WinCCOA_QualityChecks/msg/en_US.utf8/QgBase.cat

@@ -1,3 +1,8 @@
-00001,QualityGates does not run succesfully !!!
+00001,QualityGates does not run successfully !!!
 00010,Assertion return error: $1
 00011,Assertion works: $1
+00020,The function $1 is not implemented.
+00021,Start quality gate $1.
+00022,Calculate sources for quality gate $1.
+00023,Validate results for quality gate $1.
+00024,Quality gate $1 done. It takes to mee $2 [s.ms].

WinCCOA_QualityChecks/panels/WinCCOA_QualityChecks/general/Settings.pnl

@@ -606,7 +606,7 @@ T
 2
 LANG:10001 4 Save
 LANG:10000 9 Speichern
-"#uses \"classes/WinCCOA_Json\"
+"#uses \"classes/json/JsonFile\"
 
 main(mapping event)
 {
@@ -631,7 +631,7 @@ void saveChanges()
       string file_name = setting[\"changed\"] + \".json\";
       mappingRemove(setting, \"changed\");
 
-      Sl_Jsonfile jsonFile = Sl_Jsonfile(path + file_name, true);
+      JsonFile jsonFile = JsonFile(path + file_name, true);
       jsonFile.write(setting);
     }
   }

WinCCOA_QualityChecks/scripts/QualityGates/StaticTests/QgPanelsCheck.ctl

@@ -38,7 +38,9 @@ class QgStaticPanelCheck : QgBase
     if ( QgBase::setUp() )
       return -1;
 
-    throwError(makeError("", PRIO_INFO, ERR_CONTROL, 0, Qg::getId() + " will check " + this.checkedPath + PANELS_REL_PATH));
+
+    PanelCheck::setSourceDirPath(this.checkedPath);
+    PanelFile::setSourceDirPath(this.checkedPath);
     _panels.setDir(this.checkedPath + PANELS_REL_PATH);
 
     if ( !_panels.exists() )

WinCCOA_QualityChecks/scripts/QualityGates/StaticTests/QgPicturesCheck.ctl

@@ -27,7 +27,7 @@ class QgStaticCheck_Pictures : QgBase
 //--------------------------------------------------------------------------------
 //@public members
 //--------------------------------------------------------------------------------
-  public string checkedPath = PROJ_PATH;
+  public string checkedPath = PROJ_PATH + PICTURES_REL_PATH;
 
   //------------------------------------------------------------------------------
   /** @brief Function setups pictures tests.
@@ -40,7 +40,7 @@ class QgStaticCheck_Pictures : QgBase
       return -1;
 
     throwError(makeError("", PRIO_INFO, ERR_CONTROL, 0, Qg::getId() + " will check " + this.checkedPath + PICTURES_REL_PATH));
-    _pictures.setDir(this.checkedPath + PICTURES_REL_PATH);
+    _pictures.setDir(this.checkedPath);
 
     if ( !_pictures.exists() )
       setMinValidScore("QgStaticCheck_Pictures", "assert.missingPictures", "reason.missingPictures");
@@ -93,7 +93,7 @@ class QgStaticCheck_Pictures : QgBase
 /** 
   @breif main rutine to start QualityGate QgStaticCheck-Pictures
 */
-void main(string path = PROJ_PATH)
+void main(string path = PROJ_PATH + PICTURES_REL_PATH)
 {
   Qg::setId("QgStaticCheck_Pictures");
   QgStaticCheck_Pictures qg = QgStaticCheck_Pictures();

WinCCOA_QualityChecks/scripts/QualityGates/StaticTests/QgScriptsCheck.ctl

@@ -7,87 +7,109 @@
 // SPDX-License-Identifier: GPL-3.0-only
 //
 
-//--------------------------------------------------------------------------------
+//-----------------------------------------------------------------------------
 // used libraries (#uses)
+#uses "classes/ErrorHdl/OaLogger"
 #uses "classes/QualityGates/QgStaticCheck/CtrlCode/ScriptsDir"
 #uses "classes/QualityGates/Qg"
 #uses "classes/QualityGates/QgBase"
 
-//--------------------------------------------------------------------------------
+//-----------------------------------------------------------------------------
 // declare variables and constans
 
-//--------------------------------------------------------------------------------
-/*!
-  @brief Static code checker for ctrl scripts / libs
+//-----------------------------------------------------------------------------
+/** Static code checker for ctrl WinCC OA scripts / libs
 
   @author lschopp
 */
 class QgStaticCheck_Scripts : QgBase
 {
-  public string checkedPath = PROJ_PATH;
+//-----------------------------------------------------------------------------
+//@public members
+//-----------------------------------------------------------------------------
+  public string checkedPath = "";
+
+  //---------------------------------------------------------------------------
   public int setUp()
   {
     if ( QgBase::setUp() )
     {
       throwError(makeError("", PRIO_SEVERE, ERR_CONTROL, 0, "QgBase::setUp fails"));
       return -1;
     }
-    
+
     if ( Qg::getId() == "QgStaticCheck_Scripts" )
     {
-      throwError(makeError("", PRIO_INFO, ERR_CONTROL, 0, Qg::getId() + " will check " + this.checkedPath + SCRIPTS_REL_PATH));
-      _scriptsData.setDir(this.checkedPath + SCRIPTS_REL_PATH);
+      if (this.checkedPath.isEmpty())
+      {
+        this.checkedPath = PROJ_PATH + SCRIPTS_REL_PATH;
+      }
+      _scriptsData.setDir(this.checkedPath);
       _scriptsData.setType(ScriptsDataType::scripts);
     }
     else if ( Qg::getId() == "QgStaticCheck_Libs" )
     {
-      throwError(makeError("", PRIO_INFO, ERR_CONTROL, 0, Qg::getId() + " will check " + this.checkedPath + LIBS_REL_PATH));
-      _scriptsData.setDir(this.checkedPath + LIBS_REL_PATH);
+      if (this.checkedPath.isEmpty())
+      {
+        this.checkedPath = PROJ_PATH + LIBS_REL_PATH;
+      }
+      _scriptsData.setDir(this.checkedPath);
       _scriptsData.setType(ScriptsDataType::libs);
     }
 
     if ( !_scriptsData.exists() )
       setMinValidScore(Qg::getId(), "assert.missingScripts", "reason.missingScripts");
-    
+
     return 0;
   }
 
+  //---------------------------------------------------------------------------
   public int calculate()
   {
     if ( _scriptsData.exists() )
       return _scriptsData.calculate();
     else
       return 0;
   }
-
+
+  //---------------------------------------------------------------------------
   public int validate()
   {
-    if ( (Qg::getId() == "QgStaticCheck_Scripts") && (_scriptsData.getCountOfFilesRecursive() <= 0) && 
+    if ( (Qg::getId() == "QgStaticCheck_Scripts") && (_scriptsData.getCountOfFilesRecursive() <= 0) &&
           isdir(this.checkedPath + LIBS_REL_PATH) && (_scriptsData.getCountOfSubDirs() <= 0) )
     {
       // there are no scripts. Libs only and libs are checked in QgStaticCheck_Libs
       setMinValidScore("QgStaticCheck_Scripts", "assert.missingScripts", "reason.missingScripts");
       return 0;
     }
-    
+
     return _scriptsData.validate();
   }
-
 
+  //---------------------------------------------------------------------------
   public int tearDown()
   {
     _result = _scriptsData.result;
     return QgBase::tearDown();
   }
-
+
+//-----------------------------------------------------------------------------
+//@protected members
+//-----------------------------------------------------------------------------
+
+//-----------------------------------------------------------------------------
+//@private members
+//-----------------------------------------------------------------------------
   ScriptsDir _scriptsData = ScriptsDir();
 };
 
-//---------------------------------------------------------------------------------------------------------------------------------------
-/** 
-  Main rutine to start QG-Static check of scripts
+//-----------------------------------------------------------------------------
+/**
+  Main rutine to start QG-Static check of WinCC OA scripts, libs directories.
+  @param testType Checks WinCC OA scripts or libs directory
+  @param path Path to WinCCOA project to be checked. Per default this project.
 */
-void main(string testType, string path = PROJ_PATH)
+void main(string testType, string path = "")
 {
   if ( testType == "scripts" )
   {
@@ -99,10 +121,13 @@ void main(string testType, string path = PROJ_PATH)
   }
   else
   {
-    DebugN("Unknown testType", testType);
+    OaLogger logger;
+    // 00051,Parameter incorrect
+    logger.fatal(51, "testType: " + testType, "Allowed values are 'scripts', 'libs'");
+    // defensive code, shall never happens
     exit(-1);
   }
-  
+
   QgStaticCheck_Scripts qg = QgStaticCheck_Scripts();
   qg.checkedPath = path;
   exit(qg.start());