Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ANE-782] [ANE-890] Improves lexer for setup.py #1191

Merged
merged 6 commits into from
May 31, 2023

Conversation

meghfossa
Copy link
Contributor

@meghfossa meghfossa commented May 16, 2023

Overview

This PR,

  • Adds line comment lexer into setup.py parsing, so fossa-cli does not throw terminal error, when line comments are encountered
  • Adds \ lexer into setup.py parsing, so fossa-cli does not throw terminal error, when \ are encountered - i.e. line joining in python

Acceptance criteria

  • Setup.py parser ignores line comments
  • fossa-cli does not thrown an error when line comments are encountered
  • fossa-cli does not thrown an error when \ are used for line joining

Testing plan

  1. git checkout master && git pull origin && git checkout fix/setup-py-comment-parsing && make install-dev
  2. mkdir pr1191 && cd pr1191
  3. touch setup.py

for line comment

from setuptools import setup, find_packages

setup(
    name='example',
    version='0.0.1',
    description='Setting up a simple python package',
    author='some author',
    author_email='someauthor@example.com',
    url='https://example.com',
    packages=find_packages(include=['exampleproject', 'exampleproject.*']),
    install_requires=[
        'PyYAML', #
        # 'pandas==0.23.3',
        'numpy>=1.14.5',
    ],
    tests_require=['pytest'],
    entry_points={
        'console_scripts': ['my-command=exampleproject.example:main']
    },
)

for backslash

wget https://raw.githubusercontent.com/zeroknowledgediscovery/Cynet/master/setup.py
  1. fossa-dev analyze -o
  • It should not throw an error when line comments are included in install_requires block
  • It should not include dependency commented out
  • It should include all dependency when \ is used to join lines

Risks

N/A

References

Checklist

  • I added tests for this PR's change (or explained in the PR description why tests don't make sense).
  • If this PR introduced a user-visible change, I added documentation into docs/.
  • If this change is externally visible, I updated Changelog.md. If this PR did not mark a release, I added my changes into an # Unreleased section at the top.
  • If I made changes to .fossa.yml or fossa-deps.{json.yml}, I updated docs/references/files/*.schema.json. You may also need to update these if you have added/removed new dependency type (e.g. pip) or analysis target type (e.g. poetry).

@meghfossa meghfossa changed the title [ANE-???] Adds line comment Lexer for setup.py [ANE-782] [ANE-890] Adds line comment Lexer for setup.py May 16, 2023
@meghfossa meghfossa changed the title [ANE-782] [ANE-890] Adds line comment Lexer for setup.py [ANE-782] [ANE-890] Improves lexer for setup.py May 16, 2023
@meghfossa meghfossa marked this pull request as ready for review May 16, 2023 04:51
@meghfossa meghfossa requested a review from a team as a code owner May 16, 2023 04:51
@meghfossa meghfossa requested a review from csasarak May 16, 2023 04:51
@meghfossa meghfossa force-pushed the fix/setup-py-comment-parsing branch from 21a8054 to c39e1b6 Compare May 24, 2023 18:24
Copy link
Contributor

@csasarak csasarak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@csasarak
Copy link
Contributor

When it comes time to merge this, we should double-check the changelog in case 3.8 has been released already.

@meghfossa meghfossa enabled auto-merge (squash) May 31, 2023 17:39
@meghfossa meghfossa merged commit f40995c into master May 31, 2023
17 checks passed
@meghfossa meghfossa deleted the fix/setup-py-comment-parsing branch May 31, 2023 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants