Fix source root not recognized

[Julfried]

Type of Changes

	Type
✓	🐛 Bug fix
	✨ New feature
	🔨 Refactoring
	📜 Docs

Description

Closes #10026

This PR fixes a bug in the discover_package_path function in pylint/pyreverse. The function was incorrectly handling cases where the source root is a child of the module directory, causing issues when using pyreverse with certain project structures, particularly those with a src layout.

The fix modifies the condition for identifying the correct source root, allowing it to handle both cases where the source root is a parent or a child of the module directory.

Changes made:

• Updated the condition in discover_package_path function to correctly identify the source root:

  if os.path.commonpath([source_root, dirname]) in [dirname, source_root]:
      return source_root

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.80%. Comparing base (2e0c41f) to head (93f980b).
Report is 20 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #10036   +/-   ##
=======================================
  Coverage   95.80%   95.80%           
=======================================
  Files         174      174           
  Lines       18937    18937           
=======================================
  Hits        18143    18143           
  Misses        794      794           

Files with missing lines	Coverage Δ
pylint/lint/expand_modules.py	95.29% <100.00%> (ø)

[DanielNoord]

Thanks for the fix! Are you able to add a test for this as well?

[Julfried]

Thank you, I will take a look into it this week!

[DanielNoord]

Could you fix CI for the style and changeling checks? I think the PR itself looks really good already, thanks! 😄

[Julfried]

Thank you, typo should be fixed

[Pierre-Sassoulas]

Hey @Julfried thank you for fixing the typo, do you mind adding a fragment for the changelog please ?

Here: https://github.com/pylint-dev/pylint/tree/main/doc/whatsnew/fragments

(https://pylint.readthedocs.io/en/stable/development_guide/contributor_guide/contribute.html#creating-a-pull-request)

[Julfried]

Thank you. The fragment should be added!

[DanielNoord]

Thanks!

[github-actions]

🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉

This comment was generated for commit 93f980b

[echoix]

Hi!

I stumbled upon the same issue that this PR solves. Using a pylint version installed with a git url+hash through pip makes using source-roots work. Since it is not published yet, do you have any guidance on how source-roots should have been used originally? I can't find any usage pattern that would make it work at all according to the couple sentences present in the docs. I tried for hours now stepping through "/usr/local/python/3.12.7/lib/python3.12/site-packages/pylint/lint/expand_modules.py:discover_package_path()" with different combinations of invocations trying to have one that would match the one when using the developpement version (pip install git+https://github.com/pylint-dev/pylint@9b272cca71d0e1733c0aa75dc05d293ec99d44c0, uninstalling pylint before changing between released and git versions.)

If there's absolutely no workaround, can you consider backporting this specific fix to a version that would be released sooner?

[Pierre-Sassoulas]

Hi @echoix I've created #10083 to backport, waiting for the opinion of other maintainers before merging because this is a part that is hard to test and historically caused a lot of problem (maybe not well suited to a patch release). While waiting for the release you can use an install using git for example: pip install git+git://http://github.com/pylint-dev/pylint.git@main#egg=pylint -U

[echoix]

Thanks @Pierre-Sassoulas! Indeed the git version could be used, but at some point to finish off my PR I should need a release at one point.
Our codebase is stuck with a very old Pylint 2 version, and the most I was able to get it updated before breaking other jobs in the workflow was 2.12.2, but the Python version support is becoming limited. I worked against starting off from scratch with a complete new config, with Pylint 3.x.
What's hard with implementing it gradually is that contrary to flake8 and ruff, there's no way to have in a single place a list of files that ignores one or multiple rules each. That pattern is quite useful, as we can then fix gradually the issues (and we do!). For example, if an certain type of issue is in every __init__.py, we can specify it there until ready to fix, without disabling the rule for everywhere else, preventing introducing new problems. There's some plugin that seem to work in this idea, but I'm not that successful yet.

[Pierre-Sassoulas]

What I did in a project where I had this issue recently was to modify the python path with an init hook to add the source root to it init-hook='import sys; sys.path.append("/path/to/sourceroot")'. (I did not actually check if this PR fix my issue, the problem with this hack is that it ruined my motivation to actually investigate the root cause in pylint... :/ ).

[echoix]

What I did in a project where I had this issue recently was to modify the python path with an init hook to add the source root to it init-hook='import sys; sys.path.append("/path/to/sourceroot")'. (I did not actually check if this PR fix my issue, the problem with this hack is that it ruined my motivation to actually investigate the root cause in pylint... :/ ).

Ours is quite sensitive to paths, as it uses some installed library and already have difficulty making normal imports work correctly... (we're working on it). The old way called Pylint 3 times, and had to be in different directories, with settings for each, not really easy to use for every contributor.

[Pierre-Sassoulas]

What's hard with implementing it gradually is that contrary to flake8 and ruff, there's no way to have in a single place a list of files that ignores one or multiple rules each. That pattern is quite useful, as we can then fix gradually the issues (and we do!). For example, if an certain type of issue is in every init.py, we can specify it there until ready to fix, without disabling the rule for everywhere else, preventing introducing new problems. There's some plugin that seem to work in this idea, but I'm not that successful yet.

You can list disable in the pylintrc, or at the top of files and progressively delete them. Not ideal, I know. There's #5403