Add CRC validator (#5857)

* Adds crc validation checks

Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com>

* Fixes invalid CRCs

Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com>

* Changes test to smoke suite

Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com>

* excludes some test data from the validator

Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com>

* uses pathlib instead of os.path

Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com>

* fixes wrong path to test scripts

Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com>

* Escape not needed

Signed-off-by: Esteban Papp <81431996+amznestebanpapp@users.noreply.github.com>

Code/Framework/AzCore/AzCore/Math/Crc.h

@@ -16,7 +16,7 @@
 //
 // When AZ_CRC("My string") is used by default it will map to AZ::Crc32("My string").
 // We do have a pro-processor program which will precompute the crc for you and
-// transform that macro to AZ_CRC("My string",0xabcdef00) this will expand to just 0xabcdef00.
+// transform that macro to AZ_CRC("My string", 0x18fbd270) this will expand to just 0x18fbd270.
 // This will remove completely the "My string" from your executable, it will add it to a database and so on.
 // WHen you want to update the string, just change the string.
 // If you don't run the precompile step the code should still run fine, except it will be slower,
@@ -24,7 +24,7 @@
 // a constant expression.
 // For example
 // switch(id) {
-//  case AZ_CRC("My string",0xabcdef00): {} break; // this will compile fine
+//  case AZ_CRC("My string",0x18fbd270): {} break; // this will compile fine
 //  case AZ_CRC("My string"): {} break; // this will cause "error C2051: case expression not constant"
 // }
 // So it's you choice what you do, depending on your needs.

Code/Framework/AzToolsFramework/AzToolsFramework/Viewport/ActionBus.h

@@ -23,11 +23,11 @@ namespace AzToolsFramework
     /// @name Reverse URLs.
     /// Used to identify common actions and override them when necessary.
     //@{
-    static const AZ::Crc32 s_backAction = AZ_CRC("com.o3de.action.common.back", 0xd772a2af);
-    static const AZ::Crc32 s_deleteAction = AZ_CRC("com.o3de.action.common.delete", 0x5731f6cb);
-    static const AZ::Crc32 s_duplicateAction = AZ_CRC("com.o3de.action.common.duplicate", 0x08ccf461);
-    static const AZ::Crc32 s_nextComponentMode = AZ_CRC("com.o3de.action.common.nextComponentMode", 0xcc26094f);
-    static const AZ::Crc32 s_previousComponentMode = AZ_CRC("com.o3de.action.common.previousComponentMode", 0x0d18ff39);
+    static const AZ::Crc32 s_backAction = AZ_CRC("com.o3de.action.common.back", 0x80c3030f);
+    static const AZ::Crc32 s_deleteAction = AZ_CRC("com.o3de.action.common.delete", 0x58e78eed);
+    static const AZ::Crc32 s_duplicateAction = AZ_CRC("com.o3de.action.common.duplicate", 0xbc5a4a23);
+    static const AZ::Crc32 s_nextComponentMode = AZ_CRC("com.o3de.action.common.nextComponentMode", 0xf9aca3a8);
+    static const AZ::Crc32 s_previousComponentMode = AZ_CRC("com.o3de.action.common.previousComponentMode", 0x0580eaec);
     //@}
 
     /// Specific Action properties to be sent to a type implementing

Gems/AtomLyIntegration/AtomBridge/Code/Source/AtomBridgeSystemComponent.cpp

@@ -67,12 +67,12 @@ namespace AZ
 
         void AtomBridgeSystemComponent::GetProvidedServices(ComponentDescriptor::DependencyArrayType& provided)
         {
-            provided.push_back(AZ_CRC("AtomBridgeService", 0xdb816a99));
+            provided.push_back(AZ_CRC("AtomBridgeService", 0x92d990b5));
         }
 
         void AtomBridgeSystemComponent::GetIncompatibleServices(ComponentDescriptor::DependencyArrayType& incompatible)
         {
-            incompatible.push_back(AZ_CRC("AtomBridgeService", 0xdb816a99));
+            incompatible.push_back(AZ_CRC("AtomBridgeService", 0x92d990b5));
         }
 
         void AtomBridgeSystemComponent::GetRequiredServices(ComponentDescriptor::DependencyArrayType& required)

Gems/AtomLyIntegration/AtomBridge/Code/Source/Editor/AssetCollectionAsyncLoaderTestComponent.h

@@ -109,12 +109,12 @@ namespace AZ
 
             static void GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& services)
             {
-                services.push_back(AZ_CRC("AssetCollectionAsyncLoaderTest", 0xdd5ab934));
+                services.push_back(AZ_CRC("AssetCollectionAsyncLoaderTest", 0x66d04369));
             }
 
             static void GetIncompatibleServices(AZ::ComponentDescriptor::DependencyArrayType& services)
             {
-                services.push_back(AZ_CRC("AssetCollectionAsyncLoaderTest", 0xdd5ab934));
+                services.push_back(AZ_CRC("AssetCollectionAsyncLoaderTest", 0x66d04369));
             }
 
             static void Reflect(AZ::ReflectContext* context);

Gems/AtomLyIntegration/AtomViewportDisplayIcons/Code/Source/AtomViewportDisplayIconsSystemComponent.cpp

@@ -73,7 +73,7 @@ namespace AZ::Render
     void AtomViewportDisplayIconsSystemComponent::GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required)
     {
         required.push_back(AZ_CRC("RPISystem", 0xf2add773));
-        required.push_back(AZ_CRC("AtomBridgeService", 0xdb816a99));
+        required.push_back(AZ_CRC("AtomBridgeService", 0x92d990b5));
     }
 
     void AtomViewportDisplayIconsSystemComponent::GetDependentServices([[maybe_unused]] AZ::ComponentDescriptor::DependencyArrayType& dependent)

Gems/AtomLyIntegration/CommonFeatures/Code/Source/OcclusionCullingPlane/OcclusionCullingPlaneComponentController.cpp

@@ -59,12 +59,12 @@ namespace AZ
 
         void OcclusionCullingPlaneComponentController::GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& provided)
         {
-            provided.push_back(AZ_CRC("OcclusionCullingPlaneService", 0x9123f33d));
+            provided.push_back(AZ_CRC("OcclusionCullingPlaneService", 0x7d036c2e));
         }
 
         void OcclusionCullingPlaneComponentController::GetIncompatibleServices(AZ::ComponentDescriptor::DependencyArrayType& incompatible)
         {
-            incompatible.push_back(AZ_CRC("OcclusionCullingPlaneService", 0x9123f33d));
+            incompatible.push_back(AZ_CRC("OcclusionCullingPlaneService", 0x7d036c2e));
         }
 
         void OcclusionCullingPlaneComponentController::GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required)

Gems/Blast/Code/Source/Editor/EditorSystemComponent.h

@@ -31,12 +31,12 @@ namespace Blast
     private:
         static void GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& provided)
         {
-            provided.push_back(AZ_CRC("BlastEditorService", 0x0a61cda5));
+            provided.push_back(AZ_CRC("BlastEditorService", 0xeddfed0d));
         }
 
         static void GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required)
         {
-            required.push_back(AZ_CRC("BlastService", 0x75beae2d));
+            required.push_back(AZ_CRC("BlastService", 0x46927a9f));
         }
 
         AZStd::unique_ptr<EditorBlastChunksAssetHandler> m_editorBlastChunksAssetHandler;

Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/InputOutputNodePaletteItem.h

@@ -33,7 +33,7 @@ namespace GraphModelIntegration
 
         //! Constructor
         //! \param nodeName Name of the node that will show up in the Palette
-        //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0xa6d1a85a))
+        //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0x0a1dff96))
         //! \param dataType The type of data that the InputGraphNode or OutputGraphNode will represent
         InputOutputNodePaletteItem(AZStd::string_view nodeName, GraphCanvas::EditorId editorId, GraphModel::DataTypePtr dataType)
             : DraggableNodePaletteTreeItem(nodeName, editorId)

Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/ModuleNodePaletteItem.h

@@ -95,7 +95,7 @@ namespace GraphModelIntegration
         AZ_CLASS_ALLOCATOR(ModuleNodePaletteItem, AZ::SystemAllocator, 0);
 
         //! Constructor
-        //! \param editorId        Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0xa6d1a85a))
+        //! \param editorId        Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0x0a1dff96))
         //! \param sourceFileId    The unique id for the module node graph source file.
         //! \param sourceFilePath  The path to the module node graph source file. This will be used for node naming and debug output.
         ModuleNodePaletteItem(GraphCanvas::EditorId editorId, AZ::Uuid sourceFileId, AZStd::string_view sourceFilePath)

Gems/GraphModel/Code/Include/GraphModel/Integration/NodePalette/StandardNodePaletteItem.h

@@ -34,7 +34,7 @@ namespace GraphModelIntegration
 
         //! Constructor
         //! \param nodeName Name of the node that will show up in the Palette
-        //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0xa6d1a85a))
+        //! \param editorId Unique name of the client system editor (ex: AZ_CRC("ShaderCanvas", 0x0a1dff96))
         StandardNodePaletteItem(AZStd::string_view nodeName, GraphCanvas::EditorId editorId)
             : DraggableNodePaletteTreeItem(nodeName, editorId)
         {

Gems/LmbrCentral/Code/Source/Shape/EditorTubeShapeComponentMode.cpp

@@ -22,7 +22,7 @@ namespace LmbrCentral
 {
     AZ_CLASS_ALLOCATOR_IMPL(EditorTubeShapeComponentMode, AZ::SystemAllocator, 0)
 
-    static const AZ::Crc32 s_resetVariableRadii = AZ_CRC("com.o3de.action.tubeshape.reset_radii", 0x0f2ef8e2);
+    static const AZ::Crc32 s_resetVariableRadii = AZ_CRC("com.o3de.action.tubeshape.reset_radii", 0xa987659c);
     static const char* const s_resetRadiiTitle = "Reset Radii";
     static const char* const s_resetRadiiDesc = "Reset all variable radius values to the default";
 

Gems/LyShine/Code/Editor/PropertyHandlerUiParticleColorKeyframe.h

@@ -50,7 +50,7 @@ class PropertyHandlerUiParticleColorKeyframe
 public:
     AZ_CLASS_ALLOCATOR(PropertyHandlerUiParticleColorKeyframe, AZ::SystemAllocator, 0);
 
-    AZ::u32 GetHandlerName(void) const override { return AZ_CRC("UiParticleColorKeyframeCtrl", 0x8cb3a9f1); }
+    AZ::u32 GetHandlerName(void) const override { return AZ_CRC("UiParticleColorKeyframeCtrl", 0xe3ef28b6); }
     bool IsDefaultHandler() const override { return true; }
     QWidget* CreateGUI(QWidget* pParent) override;
     void ConsumeAttribute(PropertyUiParticleColorKeyframeCtrl* GUI, AZ::u32 attrib, AzToolsFramework::PropertyAttributeReader* attrValue, const char* debugName) override;

Gems/LyShine/Code/Editor/PropertyHandlerUiParticleFloatKeyframe.h

@@ -50,7 +50,7 @@ class PropertyHandlerUiParticleFloatKeyframe
 public:
     AZ_CLASS_ALLOCATOR(PropertyHandlerUiParticleFloatKeyframe, AZ::SystemAllocator, 0);
 
-    AZ::u32 GetHandlerName(void) const override { return AZ_CRC("UiParticleFloatKeyframeCtrl", 0xba9359a2); }
+    AZ::u32 GetHandlerName(void) const override { return AZ_CRC("UiParticleFloatKeyframeCtrl", 0x448a90ec); }
     bool IsDefaultHandler() const override { return true; }
     QWidget* CreateGUI(QWidget* pParent) override;
     void ConsumeAttribute(PropertyUiParticleFloatKeyframeCtrl* GUI, AZ::u32 attrib, AzToolsFramework::PropertyAttributeReader* attrValue, const char* debugName) override;

Gems/LyShine/Code/Source/LyShineLoadScreen.cpp

@@ -31,12 +31,12 @@ namespace LyShine
 
     void LyShineLoadScreenComponent::GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& provided)
     {
-        provided.emplace_back(AZ_CRC("LyShineLoadScreenService", 0xBB5EAB17));
+        provided.emplace_back(AZ_CRC("LyShineLoadScreenService", 0xbb5eab17));
     }
 
     void LyShineLoadScreenComponent::GetIncompatibleServices(AZ::ComponentDescriptor::DependencyArrayType& incompatible)
     {
-        incompatible.emplace_back(AZ_CRC("LyShineLoadScreenService", 0xBB5EAB17));
+        incompatible.emplace_back(AZ_CRC("LyShineLoadScreenService", 0xbb5eab17));
     }
 
     void LyShineLoadScreenComponent::Init()

Gems/PhysX/Code/Editor/ColliderComponentMode.cpp

@@ -27,10 +27,10 @@ namespace PhysX
     namespace
     {
         //! Uri's for shortcut actions.
-        const AZ::Crc32 SetDimensionsSubModeActionUri = AZ_CRC("com.o3de.action.physx.setdimensionssubmode", 0x77b70dd6);
-        const AZ::Crc32 SetOffsetSubModeActionUri = AZ_CRC("com.o3de.action.physx.setoffsetsubmode", 0xc06132e5);
-        const AZ::Crc32 SetRotationSubModeActionUri = AZ_CRC("com.o3de.action.physx.setrotationsubmode", 0xc4225918);
-        const AZ::Crc32 ResetSubModeActionUri = AZ_CRC("com.o3de.action.physx.resetsubmode", 0xb70b120e);
+        const AZ::Crc32 SetDimensionsSubModeActionUri = AZ_CRC("com.o3de.action.physx.setdimensionssubmode", 0x508b1781);
+        const AZ::Crc32 SetOffsetSubModeActionUri = AZ_CRC("com.o3de.action.physx.setoffsetsubmode", 0x777ac743);
+        const AZ::Crc32 SetRotationSubModeActionUri = AZ_CRC("com.o3de.action.physx.setrotationsubmode", 0xf1a8f3ff);
+        const AZ::Crc32 ResetSubModeActionUri = AZ_CRC("com.o3de.action.physx.resetsubmode", 0x599d1594);
     } // namespace
 
     AZ_CLASS_ALLOCATOR_IMPL(ColliderComponentMode, AZ::SystemAllocator, 0);

Gems/PhysXDebug/Code/Source/EditorSystemComponent.h

@@ -32,7 +32,7 @@ namespace PhysXDebug
 
         static void GetProvidedServices(AZ::ComponentDescriptor::DependencyArrayType& provided)
         {
-            provided.push_back(AZ_CRC("PhysXDebugEditorService", 0xe3dde7d8));
+            provided.push_back(AZ_CRC("PhysXDebugEditorService", 0xf8611967));
         }
 
         static void GetRequiredServices(AZ::ComponentDescriptor::DependencyArrayType& required)

Gems/WhiteBox/Code/Source/SubComponentModes/EditorWhiteBoxDefaultMode.cpp

@@ -33,8 +33,8 @@ namespace WhiteBox
         AZ::Color, cl_whiteBoxVertexIndicatorColor, AZ::Color::CreateFromRgba(0, 0, 0, 102), nullptr,
         AZ::ConsoleFunctorFlags::Null, "The color of the vertex indicator");
 
-    static const AZ::Crc32 HideEdge = AZ_CRC("com.o3de.action.whitebox.hide_edge", 0x6a60ae23);
-    static const AZ::Crc32 HideVertex = AZ_CRC("com.o3de.action.whitebox.hide_vertex", 0x4a4bd092);
+    static const AZ::Crc32 HideEdge = AZ_CRC("com.o3de.action.whitebox.hide_edge", 0x84f6a9b9);
+    static const AZ::Crc32 HideVertex = AZ_CRC("com.o3de.action.whitebox.hide_vertex", 0x5f81c937);
 
     static const char* const HideEdgeTitle = "Hide Edge";
     static const char* const HideEdgeDesc = "Hide the selected edge to merge the two connected polygons";

scripts/commit_validation/CMakeLists.txt

@@ -6,10 +6,8 @@
 #
 #
 
-# this ctest makes sure that the commit validation function
-# also runs its tests during commit validation!
 ly_add_pytest(
     NAME test_commit_validation
-    PATH ${CMAKE_CURRENT_LIST_DIR}
+    PATH ${CMAKE_CURRENT_LIST_DIR}/commit_validation/tests
+    TEST_SUITE smoke
 )
-

scripts/commit_validation/commit_validation/commit_validation.py

@@ -181,4 +181,6 @@ EXCLUDED_VALIDATION_PATTERNS = [
     'restricted/*/Tools/*RemoteControl',
     '*/user/Cache/*',
     '*/user/log/*',
+    '*/user/log_test_1/*',
+    '*/user/log_test_2/*',
 ]

scripts/commit_validation/commit_validation/tests/validators/test_crc_validator.py

@@ -0,0 +1,47 @@
+#
+# Copyright (c) Contributors to the Open 3D Engine Project.
+# For complete copyright and license terms please see the LICENSE at the root of this distribution.
+#
+# SPDX-License-Identifier: Apache-2.0 OR MIT
+#
+#
+
+import unittest
+from unittest.mock import patch, mock_open
+
+from commit_validation.tests.mocks.mock_commit import MockCommit
+from commit_validation.validators.crc_validator import CrcValidator
+
+
+class CrcValidatorTests(unittest.TestCase):
+
+    @patch('builtins.open', mock_open(read_data='This file does not contain an AZ_CRC macro'))
+    def test_fileWithNoCrc_passes(self):
+        commit = MockCommit(files=['/someCppFile.cpp'])
+        error_list = []
+        self.assertTrue(CrcValidator().run(commit, error_list))
+        self.assertEqual(len(error_list), 0, f"Unexpected errors: {error_list}")
+
+    @patch('builtins.open', mock_open(read_data='This file contains an invalid CRC macro AZ_CRC("My string", 0xabcdef00)'))
+    def test_fileWithInvalidCrc_fails(self):
+        commit = MockCommit(files=['/someCppFile.cpp'])
+        error_list = []
+        self.assertFalse(CrcValidator().run(commit, error_list))
+        self.assertNotEqual(len(error_list), 0, f"Errors were expected but none were returned.")
+
+    @patch('builtins.open', mock_open(read_data='This file contains a valid CRC macro AZ_CRC("My string", 0x18fbd270)'))
+    def test_fileWithValidCrc_fails(self):
+        commit = MockCommit(files=['/someCppFile.cpp'])
+        error_list = []
+        self.assertTrue(CrcValidator().run(commit, error_list))
+        self.assertEqual(len(error_list), 0, f"Unexpected errors: {error_list}")
+
+    @patch('builtins.open', mock_open(read_data='This file contains an invalid CRC macro AZ_CRC("My string", 0xabcdef00)'))
+    def test_fileExtensionIgnored_passes(self):
+        commit = MockCommit(files=['/someCppFile.somerandomextension'])
+        error_list = []
+        self.assertTrue(CrcValidator().run(commit, error_list))
+        self.assertEqual(len(error_list), 0, f"Unexpected errors: {error_list}")
+
+if __name__ == '__main__':
+    unittest.main()

scripts/commit_validation/commit_validation/validators/crc_validator.py

@@ -0,0 +1,48 @@
+#
+# Copyright (c) Contributors to the Open 3D Engine Project.
+# For complete copyright and license terms please see the LICENSE at the root of this distribution.
+#
+# SPDX-License-Identifier: Apache-2.0 OR MIT
+#
+#
+
+import binascii
+import fnmatch
+import pathlib
+import re
+from typing import Type, List
+
+from commit_validation.commit_validation import Commit, CommitValidator, SOURCE_FILE_EXTENSIONS, EXCLUDED_VALIDATION_PATTERNS, VERBOSE
+
+class CrcValidator(CommitValidator):
+    """A file-level validator that makes sure a file does not contain an invalid CRC"""
+
+    def run(self, commit: Commit, errors: List[str]) -> bool:
+        for file_name in commit.get_files():
+            for pattern in EXCLUDED_VALIDATION_PATTERNS:
+                if fnmatch.fnmatch(file_name, pattern):
+                    if VERBOSE: print(f'{file_name}::{self.__class__.__name__} SKIPPED - Validation pattern excluded on path.')
+                    break
+            else:
+                if pathlib.Path(file_name).suffix.lower() not in SOURCE_FILE_EXTENSIONS:
+                    if VERBOSE: print(f'{file_name}::{self.__class__.__name__} SKIPPED - File excluded based on extension.')
+                    continue
+                
+                with open(file_name, mode='r', encoding='utf8') as fh:
+                    fileContents = fh.read()
+                    matchesFound = re.findall(r'AZ_CRC\("([^"]+)",([^)]*)\)', fileContents)
+                    for element in matchesFound:
+                        stringInCode = element[0]
+                        valueInCode = element[1].strip()
+                        expectedValue = "{0:#0{1}x}".format(binascii.crc32(stringInCode.lower().encode('utf8')), 10)
+                        if expectedValue != valueInCode:
+                            error_message = str(f'{file_name}::{self.__class__.__name__} FAILED - Source file contains a CRC mismatch!\n'
+                                                f'     AZ_CRC("{stringInCode}", {valueInCode}), expected value {expectedValue}')
+                            if VERBOSE: print(error_message)
+                            errors.append(error_message)
+        return (not errors)
+
+
+def get_validator() -> Type[CrcValidator]:
+    """Returns the validator class for this module"""
+    return CrcValidator