fix(hints): properly identify source component

If there is a component(child) within a component(parent), like for test_apps, the parent
component may be wrongly identified as source component for the failed include. This may
lead to a false bug report if the parent component has component, which provides the missing
header, in requirements.

Fix this by looking for the longest matching source component directory.

Suggested-by: Ivan Grokhotkov <ivan@espressif.com>
Signed-off-by: Frantisek Hrbata <frantisek.hrbata@espressif.com>

tools/idf_py_actions/hint_modules/component_requirements.py

@@ -64,9 +64,10 @@ def generate_hint(output: str) -> Optional[str]:
         # from header file, which is not present in component_info['sources']
         component_dir = os.path.normpath(component_info['dir'])
         if source_filename.startswith(component_dir):
-            found_source_component_name = component_name
+            if found_source_component_info and len(found_source_component_info['dir']) >= len(component_dir):
+                continue
             found_source_component_info = component_info
-            break
+            found_source_component_name = component_name
     if not found_source_component_name:
         # The source file is not in any component.
         # It could be in a subproject added via ExternalProject_Add, in which case

tools/test_idf_py/test_hints.py

@@ -48,16 +48,17 @@ class TestHintsMassages(unittest.TestCase):
         self.tmpdir.cleanup()
 
 
-class TestHintModuleComponentRequirements(unittest.TestCase):
-    def run_idf(self, args: List[str]) -> str:
+def run_idf(args: List[str], cwd: Path) -> str:
     # Simple helper to run idf command and return it's stdout.
     cmd = [
         sys.executable,
         os.path.join(os.environ['IDF_PATH'], 'tools', 'idf.py')
     ]
-        proc = run(cmd + args, capture_output=True, cwd=str(self.projectdir), text=True)
-        return proc.stdout + proc.stderr
+    proc = run(cmd + args, capture_output=True, cwd=cwd, text=True)
+    return str(proc.stdout + proc.stderr)
 
+
+class TestHintModuleComponentRequirements(unittest.TestCase):
     def setUp(self) -> None:
         # Set up a dummy project in tmp directory with main and component1 component.
         # The main component includes component1.h from component1, but the header dir is
@@ -87,44 +88,88 @@ class TestHintModuleComponentRequirements(unittest.TestCase):
         # The main component uses component1.h, but this header is not in component1 public
         # interface. Hints should suggest that component1.h should be added into INCLUDE_DIRS
         # of component1.
-        output = self.run_idf(['app'])
+        output = run_idf(['app'], self.projectdir)
         self.assertIn('Missing "component1.h" file name found in the following component(s): component1(', output)
 
         # Based on previous hint the component1.h is added to INCLUDE_DIRS, but main still doesn't
         # have dependency on compoment1. Hints should suggest to add component1 into main component
         # PRIV_REQUIRES, because foo.h is not in main public interface.
-        self.run_idf(['fullclean'])
+        run_idf(['fullclean'], self.projectdir)
         component1cmake = self.projectdir / 'components' / 'component1' / 'CMakeLists.txt'
         component1cmake.write_text('idf_component_register(INCLUDE_DIRS ".")')
-        output = self.run_idf(['app'])
+        output = run_idf(['app'], self.projectdir)
         self.assertIn('To fix this, add component1 to PRIV_REQUIRES list of idf_component_register call', output)
 
         # Add foo.h into main public interface. Now the hint should suggest to use
         # REQUIRES instead of PRIV_REQUIRES.
-        self.run_idf(['fullclean'])
+        run_idf(['fullclean'], self.projectdir)
         maincmake = self.projectdir / 'main' / 'CMakeLists.txt'
         maincmake.write_text(('idf_component_register(SRCS "foo.c" '
                               'REQUIRES esp_timer '
                               'INCLUDE_DIRS ".")'))
-        output = self.run_idf(['app'])
+        output = run_idf(['app'], self.projectdir)
         self.assertIn('To fix this, add component1 to REQUIRES list of idf_component_register call', output)
 
         # Add component1 to REQUIRES as suggested by previous hint, but also
         # add esp_psram as private req for component1 and add esp_psram.h
-        # to component1.h. New the hint should report that esp_psram should
+        # to component1.h. Now the hint should report that esp_psram should
         # be moved from PRIV_REQUIRES to REQUIRES for component1.
-        self.run_idf(['fullclean'])
+        run_idf(['fullclean'], self.projectdir)
         maincmake.write_text(('idf_component_register(SRCS "foo.c" '
                               'REQUIRES esp_timer component1 '
                               'INCLUDE_DIRS ".")'))
         (self.projectdir / 'components' / 'component1' / 'component1.h').write_text('#include "esp_psram.h"')
         component1cmake.write_text('idf_component_register(INCLUDE_DIRS "." PRIV_REQUIRES esp_psram)')
-        output = self.run_idf(['app'])
+        output = run_idf(['app'], self.projectdir)
         self.assertIn('To fix this, move esp_psram from PRIV_REQUIRES into REQUIRES', output)
 
     def tearDown(self) -> None:
         self.tmpdir.cleanup()
 
 
+class TestNestedModuleComponentRequirements(unittest.TestCase):
+    def setUp(self) -> None:
+        # Set up a nested component structure. The components directory contains
+        # component1, which also contains foo project with main component.
+        # components/component1/project/main
+        #            ^^^^^^^^^^         ^^^^
+        #            component          nested component
+        # Both components include esp_timer.h, but only component1 has esp_timer
+        # in requirements.
+        self.tmpdir = tempfile.TemporaryDirectory()
+        self.tmpdirpath = Path(self.tmpdir.name)
+
+        components = self.tmpdirpath / 'components'
+        maindir = components / 'component1'
+        maindir.mkdir(parents=True)
+        (maindir / 'CMakeLists.txt').write_text('idf_component_register(SRCS "component1.c" PRIV_REQUIRES esp_timer)')
+        (maindir / 'component1.c').write_text('#include "esp_timer.h"')
+
+        self.projectdir = maindir / 'project'
+        self.projectdir.mkdir(parents=True)
+        (self.projectdir / 'CMakeLists.txt').write_text((
+            'cmake_minimum_required(VERSION 3.16)\n'
+            f'set(EXTRA_COMPONENT_DIRS "{components}")\n'
+            'set(COMPONENTS main)\n'
+            'include($ENV{IDF_PATH}/tools/cmake/project.cmake)\n'
+            'project(foo)'))
+
+        maindir = self.projectdir / 'main'
+        maindir.mkdir()
+        (maindir / 'CMakeLists.txt').write_text('idf_component_register(SRCS "foo.c" REQUIRES component1)')
+        (maindir / 'foo.c').write_text('#include "esp_timer.h"\nvoid app_main(){}')
+
+    def test_nested_component_requirements(self) -> None:
+        # Verify that source component for a failed include is properly identified
+        # when components are nested. The main component should be identified as the
+        # real source, not the component1 component.
+        output = run_idf(['app'], self.projectdir)
+        self.assertNotIn('BUG: esp_timer.h found in component esp_timer which is already in the requirements list of component1', output)
+        self.assertIn('To fix this, add esp_timer to PRIV_REQUIRES list of idf_component_register call', output)
+
+    def tearDown(self) -> None:
+        self.tmpdir.cleanup()
+
+
 if __name__ == '__main__':
     unittest.main()