From 49c2e57908143e2a39344e6299137c6a88a95e43 Mon Sep 17 00:00:00 2001 From: Nastya Birillo Date: Sat, 17 Jul 2021 00:03:34 +0300 Subject: [PATCH 1/4] Fix GitHub actions (#46) * Update requirements and dockerfile * Use Dockerfile in Github Action * Ignore a part of flake8 issues * Ignore flake8 inspections * Separate docker file into two: for prod and for dev * Delete ` * Fix test_range_of_lines * Fix pylint and flake8 tests * Fix pylint tests * Check styles by flake8 * Fix C408 issue * Move production Dockerfile * Fix Dockefile for production * Set up Eslint in the DockerFile for production * Delete one case from flake8 tests * Add pathlib into whitelist * Delete Path from setup.py * Ignore pathlib * Don't count INFO issues * Fix flake8 tests * Small fix * Delete pathlib from the whitelist * Update docker (#54) * Use one docker file for prod and dev * Change base docker image * Fix requirements-test: delete duplicates, setup versions * Try to fix dockerfile * Fix run in subprocess function * Fix Dockerfile * Try to fix Dockerfile * Fix docker image (#58) * Change eslint installing * Update build.yaml * Introduce base docker image and auto-detect eslint path Co-authored-by: Andrey Balandin Closes #43, closes #45. --- .dockerignore | 7 + .github/workflows/build.yml | 61 +++---- Dockerfile | 34 +--- Dockerfile.base | 170 ++++++++++++++++++ README.md | 2 +- requirements-test.txt | 11 +- requirements.txt | 25 +-- src/python/review/inspectors/detekt/detekt.py | 2 +- src/python/review/inspectors/eslint/eslint.py | 9 +- .../review/inspectors/flake8/issue_types.py | 2 +- src/python/review/inspectors/issue.py | 1 + src/python/review/reviewers/perform_review.py | 4 +- .../functional_tests/test_range_of_lines.py | 28 +-- .../inspectors/test_eslint_inspector.py | 2 +- .../inspectors/test_flake8_inspector.py | 14 +- test/python/inspectors/test_local_review.py | 2 +- .../inspectors/test_pylint_inspector.py | 4 +- 17 files changed, 265 insertions(+), 113 deletions(-) create mode 100644 .dockerignore create mode 100644 Dockerfile.base diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 00000000..3f78c748 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,7 @@ +**/*.pyc +**/.pytest_cache/ +**/__pycache__/ +.git +.github +.idea +node_modules diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 6c56547d..54fe8f08 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -1,43 +1,34 @@ name: Python build -on: [push, pull_request] +on: [ push, pull_request ] jobs: - build: + build: runs-on: ubuntu-latest - strategy: - matrix: - python-version: [3.8] + # Consistent with base image in Dockerfile + container: stepik/hyperstyle-base:py3.8.11-java11.0.11-node14.17.3 steps: - - uses: actions/checkout@v2 - - name: Set up Python ${{ matrix.python-version }} - uses: actions/setup-python@v2 - with: - python-version: ${{ matrix.python-version }} - - name: Install dependencies - run: | - python -m pip install --upgrade pip - pip install flake8 pytest - pip install -r requirements.txt - pip install -r requirements-test.txt - - name: Lint with flake8 - run: | - # stop the build if there are Python syntax errors or undefined names - flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules - # TODO: change max-complexity into 10 after refactoring - flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules - - name: Set up Eslint - run: | - npm install eslint --save-dev - ./node_modules/.bin/eslint --init - - name: Set up Java ${{ matrix.python-version }} - uses: actions/setup-java@v1 - with: - java-version: '11' - - name: Check java version - run: java -version - - name: Test with pytest - run: | - pytest \ No newline at end of file + - name: Checkout + uses: actions/checkout@v1 + + - name: Install requirements + run: | + pip install --no-cache-dir -r requirements-test.txt -r requirements.txt + + - name: Lint with flake8 + run: | + # stop the build if there are Python syntax errors or undefined names + flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules + # TODO: change max-complexity into 10 after refactoring + flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules + + - name: Set up Eslint + run: | + # Consistent with eslint version in Dockerfile + npm install eslint@7.5.0 -g && eslint --init + + - name: Test with pytest + run: | + pytest diff --git a/Dockerfile b/Dockerfile index 0b9fd5a9..b33c5f72 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,30 +1,12 @@ -FROM python:3.8.2-alpine3.11 +FROM stepik/hyperstyle-base:py3.8.11-java11.0.11-node14.17.3 -RUN apk --no-cache add openjdk11 --repository=http://dl-cdn.alpinelinux.org/alpine/edge/community \ - && apk add --update nodejs npm - -RUN npm i -g eslint@7.5.0 - -RUN java -version -RUN ls /usr/lib/jvm - -# Other dependencies -RUN apk add bash - -# Dependencies and package installation -WORKDIR / - -COPY requirements-test.txt review/requirements-test.txt -RUN pip3 install --no-cache-dir -r review/requirements-test.txt - -COPY requirements.txt review/requirements.txt -RUN pip3 install --no-cache-dir -r review/requirements.txt +RUN npm install eslint@7.5.0 -g \ + && eslint --init COPY . review -RUN pip3 install --no-cache-dir ./review - -# Container's enviroment variables -ENV JAVA_HOME=/usr/lib/jvm/java-11-openjdk -ENV PATH="$JAVA_HOME/bin:${PATH}"` +RUN pip install --no-cache-dir \ + -r review/requirements-test.txt \ + -r review/requirements.txt \ + ./review -CMD ["bin/bash"] +CMD ["/bin/bash"] diff --git a/Dockerfile.base b/Dockerfile.base new file mode 100644 index 00000000..c6869724 --- /dev/null +++ b/Dockerfile.base @@ -0,0 +1,170 @@ +FROM python:3.8.11-slim + +######### +# Taken from https://github.com/docker-library/openjdk/blob/608f26c5ea63ca34070b439c904cb94a30f6b0c1/11/jdk/slim-buster/Dockerfile +######### + +RUN set -eux; \ + apt-get update; \ + apt-get install -y --no-install-recommends \ +# utilities for keeping Debian and OpenJDK CA certificates in sync + ca-certificates p11-kit \ + ; \ + rm -rf /var/lib/apt/lists/* + +ENV JAVA_HOME /usr/local/openjdk-11 +RUN { echo '#/bin/sh'; echo 'echo "$JAVA_HOME"'; } > /usr/local/bin/docker-java-home && chmod +x /usr/local/bin/docker-java-home && [ "$JAVA_HOME" = "$(docker-java-home)" ] # backwards compatibility +ENV PATH $JAVA_HOME/bin:$PATH + +# Default to UTF-8 file.encoding +ENV LANG C.UTF-8 + +# https://adoptopenjdk.net/upstream.html +# > +# > What are these binaries? +# > +# > These binaries are built by Red Hat on their infrastructure on behalf of the OpenJDK jdk8u and jdk11u projects. The binaries are created from the unmodified source code at OpenJDK. Although no formal support agreement is provided, please report any bugs you may find to https://bugs.java.com/. +# > +ENV JAVA_VERSION 11.0.11+9 +# https://github.com/docker-library/openjdk/issues/320#issuecomment-494050246 +# > +# > I am the OpenJDK 8 and 11 Updates OpenJDK project lead. +# > ... +# > While it is true that the OpenJDK Governing Board has not sanctioned those releases, they (or rather we, since I am a member) didn't sanction Oracle's OpenJDK releases either. As far as I am aware, the lead of an OpenJDK project is entitled to release binary builds, and there is clearly a need for them. +# > + +RUN set -eux; \ + \ + arch="$(dpkg --print-architecture)"; \ + case "$arch" in \ + 'amd64') \ + downloadUrl='https://github.com/AdoptOpenJDK/openjdk11-upstream-binaries/releases/download/jdk-11.0.11%2B9/OpenJDK11U-jdk_x64_linux_11.0.11_9.tar.gz'; \ + ;; \ + 'arm64') \ + downloadUrl='https://github.com/AdoptOpenJDK/openjdk11-upstream-binaries/releases/download/jdk-11.0.11%2B9/OpenJDK11U-jdk_aarch64_linux_11.0.11_9.tar.gz'; \ + ;; \ + *) echo >&2 "error: unsupported architecture: '$arch'"; exit 1 ;; \ + esac; \ + \ + savedAptMark="$(apt-mark showmanual)"; \ + apt-get update; \ + apt-get install -y --no-install-recommends \ + dirmngr \ + gnupg \ + wget \ + ; \ + rm -rf /var/lib/apt/lists/*; \ + \ + wget --progress=dot:giga -O openjdk.tgz "$downloadUrl"; \ + wget --progress=dot:giga -O openjdk.tgz.asc "$downloadUrl.sign"; \ + \ + export GNUPGHOME="$(mktemp -d)"; \ +# pre-fetch Andrew Haley's (the OpenJDK 8 and 11 Updates OpenJDK project lead) key so we can verify that the OpenJDK key was signed by it +# (https://github.com/docker-library/openjdk/pull/322#discussion_r286839190) +# we pre-fetch this so that the signature it makes on the OpenJDK key can survive "import-clean" in gpg + gpg --batch --keyserver keyserver.ubuntu.com --recv-keys EAC843EBD3EFDB98CC772FADA5CD6035332FA671; \ +# TODO find a good link for users to verify this key is right (https://mail.openjdk.java.net/pipermail/jdk-updates-dev/2019-April/000951.html is one of the only mentions of it I can find); perhaps a note added to https://adoptopenjdk.net/upstream.html would make sense? +# no-self-sigs-only: https://salsa.debian.org/debian/gnupg2/commit/c93ca04a53569916308b369c8b218dad5ae8fe07 + gpg --batch --keyserver keyserver.ubuntu.com --keyserver-options no-self-sigs-only --recv-keys CA5F11C6CE22644D42C6AC4492EF8D39DC13168F; \ + gpg --batch --list-sigs --keyid-format 0xLONG CA5F11C6CE22644D42C6AC4492EF8D39DC13168F \ + | tee /dev/stderr \ + | grep '0xA5CD6035332FA671' \ + | grep 'Andrew Haley'; \ + gpg --batch --verify openjdk.tgz.asc openjdk.tgz; \ + gpgconf --kill all; \ + rm -rf "$GNUPGHOME"; \ + \ + mkdir -p "$JAVA_HOME"; \ + tar --extract \ + --file openjdk.tgz \ + --directory "$JAVA_HOME" \ + --strip-components 1 \ + --no-same-owner \ + ; \ + rm openjdk.tgz*; \ + \ + apt-mark auto '.*' > /dev/null; \ + [ -z "$savedAptMark" ] || apt-mark manual $savedAptMark > /dev/null; \ + apt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false; \ + \ +# update "cacerts" bundle to use Debian's CA certificates (and make sure it stays up-to-date with changes to Debian's store) +# see https://github.com/docker-library/openjdk/issues/327 +# http://rabexc.org/posts/certificates-not-working-java#comment-4099504075 +# https://salsa.debian.org/java-team/ca-certificates-java/blob/3e51a84e9104823319abeb31f880580e46f45a98/debian/jks-keystore.hook.in +# https://git.alpinelinux.org/aports/tree/community/java-cacerts/APKBUILD?id=761af65f38b4570093461e6546dcf6b179d2b624#n29 + { \ + echo '#!/usr/bin/env bash'; \ + echo 'set -Eeuo pipefail'; \ + echo 'trust extract --overwrite --format=java-cacerts --filter=ca-anchors --purpose=server-auth "$JAVA_HOME/lib/security/cacerts"'; \ + } > /etc/ca-certificates/update.d/docker-openjdk; \ + chmod +x /etc/ca-certificates/update.d/docker-openjdk; \ + /etc/ca-certificates/update.d/docker-openjdk; \ + \ +# https://github.com/docker-library/openjdk/issues/331#issuecomment-498834472 + find "$JAVA_HOME/lib" -name '*.so' -exec dirname '{}' ';' | sort -u > /etc/ld.so.conf.d/docker-openjdk.conf; \ + ldconfig; \ + \ +# https://github.com/docker-library/openjdk/issues/212#issuecomment-420979840 +# https://openjdk.java.net/jeps/341 + java -Xshare:dump; \ + \ +# basic smoke test + fileEncoding="$(echo 'System.out.println(System.getProperty("file.encoding"))' | jshell -s -)"; [ "$fileEncoding" = 'UTF-8' ]; rm -rf ~/.java; \ + javac --version; \ + java --version + +######### +# Taken from https://github.com/nodejs/docker-node/blob/fd130acf063b312355a5d88d51716db3ff34ae49/14/buster-slim/Dockerfile +######### + +ENV NODE_VERSION 14.17.3 + +RUN ARCH= && dpkgArch="$(dpkg --print-architecture)" \ + && case "${dpkgArch##*-}" in \ + amd64) ARCH='x64';; \ + ppc64el) ARCH='ppc64le';; \ + s390x) ARCH='s390x';; \ + arm64) ARCH='arm64';; \ + armhf) ARCH='armv7l';; \ + i386) ARCH='x86';; \ + *) echo "unsupported architecture"; exit 1 ;; \ + esac \ + && set -ex \ + # libatomic1 for arm + && apt-get update && apt-get install -y ca-certificates curl wget gnupg dirmngr xz-utils libatomic1 --no-install-recommends \ + && rm -rf /var/lib/apt/lists/* \ + && for key in \ + 4ED778F539E3634C779C87C6D7062848A1AB005C \ + 94AE36675C464D64BAFA68DD7434390BDBE9B9C5 \ + 74F12602B6F1C4E913FAA37AD3A89613643B6201 \ + 71DCFD284A79C3B38668286BC97EC7A07EDE3FC1 \ + 8FCCA13FEF1D0C2E91008E09770F7A9A5AE15600 \ + C4F0DFFF4E8C1A8236409D08E73BC641CC11F4C8 \ + C82FA3AE1CBEDC6BE46B9360C43CEC45C17AB93C \ + DD8F2338BAE7501E3DD5AC78C273792F7D83545D \ + A48C2BEE680E841632CD4E44F07496B3EB3C1762 \ + 108F52B48DB57BB0CC439B2997B01419BD92F80A \ + B9E2F5981AA6E0CD28160D9FF13993A75599653C \ + ; do \ + gpg --batch --keyserver hkps://keys.openpgp.org --recv-keys "$key" || \ + gpg --batch --keyserver keyserver.ubuntu.com --recv-keys "$key" ; \ + done \ + && curl -fsSLO --compressed "https://nodejs.org/dist/v$NODE_VERSION/node-v$NODE_VERSION-linux-$ARCH.tar.xz" \ + && curl -fsSLO --compressed "https://nodejs.org/dist/v$NODE_VERSION/SHASUMS256.txt.asc" \ + && gpg --batch --decrypt --output SHASUMS256.txt SHASUMS256.txt.asc \ + && grep " node-v$NODE_VERSION-linux-$ARCH.tar.xz\$" SHASUMS256.txt | sha256sum -c - \ + && tar -xJf "node-v$NODE_VERSION-linux-$ARCH.tar.xz" -C /usr/local --strip-components=1 --no-same-owner \ + && rm "node-v$NODE_VERSION-linux-$ARCH.tar.xz" SHASUMS256.txt.asc SHASUMS256.txt \ + && apt-mark auto '.*' > /dev/null \ + && find /usr/local -type f -executable -exec ldd '{}' ';' \ + | awk '/=>/ { print $(NF-1) }' \ + | sort -u \ + | xargs -r dpkg-query --search \ + | cut -d: -f1 \ + | sort -u \ + | xargs -r apt-mark manual \ + && apt-get purge -y --auto-remove -o APT::AutoRemove::RecommendsImportant=false \ + && ln -s /usr/local/bin/node /usr/local/bin/nodejs \ + # smoke tests + && node --version \ + && npm --version diff --git a/README.md b/README.md index 84a74445..4f9856dc 100644 --- a/README.md +++ b/README.md @@ -194,7 +194,7 @@ __Note__: If you have `ModuleNotFoundError` while you try to run tests, please c __Note__: We use [eslint](https://eslint.org/) and [open-jdk 11](https://openjdk.java.net/projects/jdk/11/) in the tests. Please, set up the environment before running the tests. You can see en example of the environment configuration in -the [build.yml](./.github/workflows/build.yml) file. +the [Dockerfile](Dockerfile) file. Use `pytest` from the root directory to run __ALL__ tests. diff --git a/requirements-test.txt b/requirements-test.txt index d0ec886e..c2bf26e0 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -1,8 +1,5 @@ -pytest~=5.4.3 -pytest-runner -pytest-subtests +pytest==6.2.3 +pytest-runner==5.2 +pytest-subtests==0.4.0 jsonschema==3.2.0 -Django~=3.0.8 -pylint~=2.5.3 -requests~=2.24.0 -setuptools~=47.3.1 \ No newline at end of file +pandas==1.2.3 \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index 6fe19370..a859e972 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,23 +1,24 @@ -setuptools==47.3.1 +setuptools==56.0.0 # python code analysis tools -pylint==2.5.3 -pylint-django==2.0.15 -flake8==3.8.3 +pylint==2.7.4 +pylint-django==2.3.0 +flake8==3.9.0 # flake8 plugins -flake8-bugbear==20.1.4 +flake8-bugbear==21.4.3 flake8-builtins==1.5.3 -flake8-comprehensions==3.2.3 -flake8-eradicate==0.4.0 +flake8-comprehensions==3.4.0 +flake8-eradicate==1.0.0 flake8-import-order==0.18.1 -flake8-plugin-utils==1.3.0 +flake8-plugin-utils==1.3.1 flake8-polyfill==1.0.2 -flake8-return==1.1.1 -flake8-spellcheck==0.14.0 +flake8-return==1.1.2 +flake8-spellcheck==0.24.0 mccabe==0.6.1 pep8-naming==0.11.1 # extra libraries and frameworks -django==3.0.8 -requests==2.24.0 \ No newline at end of file +django==3.2 +requests==2.25.1 +argparse==1.4.0 \ No newline at end of file diff --git a/src/python/review/inspectors/detekt/detekt.py b/src/python/review/inspectors/detekt/detekt.py index 2c2b197e..741a334d 100644 --- a/src/python/review/inspectors/detekt/detekt.py +++ b/src/python/review/inspectors/detekt/detekt.py @@ -41,7 +41,7 @@ def _create_command(cls, path: Path, output_path: Path): '--input', str(path) ] - def inspect(self, path: Path, config) -> List[BaseIssue]: + def inspect(self, path: Path, config: dict) -> List[BaseIssue]: with new_temp_dir() as temp_dir: output_path = temp_dir / 'output.xml' command = self._create_command(path, output_path) diff --git a/src/python/review/inspectors/eslint/eslint.py b/src/python/review/inspectors/eslint/eslint.py index ff9f9a67..19d801cf 100644 --- a/src/python/review/inspectors/eslint/eslint.py +++ b/src/python/review/inspectors/eslint/eslint.py @@ -21,8 +21,9 @@ class ESLintInspector(BaseInspector): } @classmethod - def _create_command(cls, path: Path, output_path: Path, is_local: bool = False) -> List[str]: - eslint_command = 'eslint' if not is_local else './node_modules/.bin/eslint' + def _create_command(cls, path: Path, output_path: Path) -> List[str]: + local_path = 'node_modules/.bin/eslint' # used only in local dev environment + eslint_command = local_path if Path(local_path).exists() else 'eslint' return [ eslint_command, '-c', PATH_ESLINT_CONFIG, @@ -31,10 +32,10 @@ def _create_command(cls, path: Path, output_path: Path, is_local: bool = False) path, ] - def inspect(self, path: Path, config: dict, is_local: bool = False) -> List[BaseIssue]: + def inspect(self, path: Path, config: dict) -> List[BaseIssue]: with new_temp_dir() as temp_dir: output_path = temp_dir / 'output.xml' - command = self._create_command(path, output_path, is_local) + command = self._create_command(path, output_path) run_in_subprocess(command) issues = parse_checkstyle_file_result(output_path, diff --git a/src/python/review/inspectors/flake8/issue_types.py b/src/python/review/inspectors/flake8/issue_types.py index e98a4836..7997d32c 100644 --- a/src/python/review/inspectors/flake8/issue_types.py +++ b/src/python/review/inspectors/flake8/issue_types.py @@ -29,5 +29,5 @@ 'F': IssueType.BEST_PRACTICES, # standard flake8 'C': IssueType.BEST_PRACTICES, # flake8-comprehensions - 'SC': IssueType.BEST_PRACTICES, # flake8-spellcheck + 'SC': IssueType.INFO, # flake8-spellcheck } diff --git a/src/python/review/inspectors/issue.py b/src/python/review/inspectors/issue.py index 14f0bebb..3174d7c6 100644 --- a/src/python/review/inspectors/issue.py +++ b/src/python/review/inspectors/issue.py @@ -25,6 +25,7 @@ class IssueType(Enum): COHESION = 'COHESION' CLASS_RESPONSE = 'CLASS_RESPONSE' METHOD_NUMBER = 'METHOD_NUMBER' + INFO = 'INFO' # Keys in results dictionary diff --git a/src/python/review/reviewers/perform_review.py b/src/python/review/reviewers/perform_review.py index 678e503a..277b5187 100644 --- a/src/python/review/reviewers/perform_review.py +++ b/src/python/review/reviewers/perform_review.py @@ -6,6 +6,7 @@ from src.python.review.application_config import ApplicationConfig from src.python.review.common.language import Language +from src.python.review.inspectors.issue import IssueType from src.python.review.reviewers.common import perform_language_review from src.python.review.reviewers.python import perform_python_review from src.python.review.reviewers.review_result import ReviewResult @@ -66,7 +67,8 @@ def perform_and_print_review(path: Path, else: print_review_result_as_text(review_result, path) - return len(review_result.all_issues) + # Don't count INFO issues too + return len(list(filter(lambda issue: issue.type != IssueType.INFO, review_result.all_issues))) def perform_review(path: Path, config: ApplicationConfig) -> ReviewResult: diff --git a/test/python/functional_tests/test_range_of_lines.py b/test/python/functional_tests/test_range_of_lines.py index bcadae0b..21710909 100644 --- a/test/python/functional_tests/test_range_of_lines.py +++ b/test/python/functional_tests/test_range_of_lines.py @@ -14,23 +14,23 @@ }, 'issues': [{ 'category': 'CODE_STYLE', - 'code': 'C0326', + 'code': 'E225', 'column_number': 2, 'line': 'a=10', 'line_number': 1, - 'text': 'Exactly one space required around assignment'}, + 'text': 'missing whitespace around operator'}, {'category': 'CODE_STYLE', - 'code': 'C0326', + 'code': 'E225', 'column_number': 2, 'line': 'b=20', 'line_number': 2, - 'text': 'Exactly one space required around assignment'}, + 'text': 'missing whitespace around operator'}, {'category': 'CODE_STYLE', - 'code': 'C0326', + 'code': 'E225', 'column_number': 2, 'line': 'c=a + b', 'line_number': 4, - 'text': 'Exactly one space required around assignment' + 'text': 'missing whitespace around operator' } ] } @@ -80,8 +80,8 @@ def test_range_filter_when_start_line_is_not_first( 'code': 'MODERATE', 'text': 'Code quality (beta): MODERATE'}, 'issues': [{ - 'code': 'C0326', - 'text': 'Exactly one space required around assignment', + 'code': 'E225', + 'text': 'missing whitespace around operator', 'line': 'c=a + b', 'line_number': 4, 'column_number': 2, @@ -148,8 +148,8 @@ def test_range_filter_when_end_line_is_first( 'text': 'Code quality (beta): MODERATE' }, 'issues': [{ - 'code': 'C0326', - 'text': 'Exactly one space required around assignment', + 'code': 'E225', + 'text': 'missing whitespace around operator', 'line': 'a=10', 'line_number': 1, 'column_number': 2, @@ -214,15 +214,15 @@ def test_range_filter_when_both_start_and_end_lines_specified_not_equal_borders( 'text': 'Code quality (beta): BAD' }, 'issues': [{ - 'code': 'C0326', - 'text': 'Exactly one space required around assignment', + 'code': 'E225', + 'text': 'missing whitespace around operator', 'line': 'b=20', 'line_number': 2, 'column_number': 2, 'category': 'CODE_STYLE' }, { - 'code': 'C0326', - 'text': 'Exactly one space required around assignment', + 'code': 'E225', + 'text': 'missing whitespace around operator', 'line': 'c=a + b', 'line_number': 4, 'column_number': 2, diff --git a/test/python/inspectors/test_eslint_inspector.py b/test/python/inspectors/test_eslint_inspector.py index 076c76a0..f6681c90 100644 --- a/test/python/inspectors/test_eslint_inspector.py +++ b/test/python/inspectors/test_eslint_inspector.py @@ -19,7 +19,7 @@ def test_file_with_issues(file_name: str, n_issues: int): path_to_file = JS_DATA_FOLDER / file_name with use_file_metadata(path_to_file) as file_metadata: - issues = inspector.inspect(file_metadata.path, {}, True) + issues = inspector.inspect(file_metadata.path, {}) issues = filter_low_measure_issues(issues, Language.JS) assert len(issues) == n_issues diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 7de27bc3..56e02aec 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -14,7 +14,7 @@ ('case3_redefining_builtin.py', 1), ('case4_naming.py', 10), ('case5_returns.py', 1), - ('case6_unused_variables.py', 3), + # ('case6_unused_variables.py', 3), ('case8_good_class.py', 0), ('case7_empty_lines.py', 0), ('case10_unused_variable_in_loop.py', 1), @@ -24,7 +24,7 @@ ('case14_returns_errors.py', 4), ('case16_comments.py', 0), ('case17_dangerous_default_value.py', 1), - ('case18_comprehensions.py', 10), + ('case18_comprehensions.py', 9), ('case19_bad_indentation.py', 3), ('case21_imports.py', 2), ('case25_django.py', 0), @@ -49,9 +49,9 @@ def test_file_with_issues(file_name: str, n_issues: int): ('case2_boolean_expressions.py', IssuesTestInfo(n_code_style=1, n_cc=8)), ('case3_redefining_builtin.py', IssuesTestInfo(n_error_prone=1)), - ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_best_practices=3, n_cc=5)), - ('case6_unused_variables.py', IssuesTestInfo(n_best_practices=3, - n_cc=1)), + ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_best_practices=0, n_cc=5)), + # ('case6_unused_variables.py', IssuesTestInfo(n_best_practices=3, + # n_cc=1)), ('case8_good_class.py', IssuesTestInfo(n_cc=1)), ('case7_empty_lines.py', IssuesTestInfo(n_cc=4)), ('case10_unused_variable_in_loop.py', IssuesTestInfo(n_best_practices=1, @@ -90,13 +90,13 @@ def test_parse(): assert [issue.description for issue in issues] == ['test 1', 'test 2', 'test 3'] assert [issue.type for issue in issues] == [IssueType.CODE_STYLE, IssueType.CODE_STYLE, - IssueType.BEST_PRACTICES] + IssueType.INFO] def test_choose_issue_type(): error_codes = ['B006', 'SC100', 'R503', 'ABC123', 'E101'] expected_issue_types = [ - IssueType.ERROR_PRONE, IssueType.BEST_PRACTICES, + IssueType.ERROR_PRONE, IssueType.INFO, IssueType.ERROR_PRONE, IssueType.BEST_PRACTICES, IssueType.CODE_STYLE ] diff --git a/test/python/inspectors/test_local_review.py b/test/python/inspectors/test_local_review.py index f32caab3..8f9491a0 100644 --- a/test/python/inspectors/test_local_review.py +++ b/test/python/inspectors/test_local_review.py @@ -24,7 +24,7 @@ def config() -> ApplicationConfig: disabled_inspectors={InspectorType.INTELLIJ}, allow_duplicates=False, n_cpu=1, - inspectors_config=dict(n_cpu=1) + inspectors_config={'n_cpu': 1} ) diff --git a/test/python/inspectors/test_pylint_inspector.py b/test/python/inspectors/test_pylint_inspector.py index 1ee00735..e1da830a 100644 --- a/test/python/inspectors/test_pylint_inspector.py +++ b/test/python/inspectors/test_pylint_inspector.py @@ -6,7 +6,7 @@ from .conftest import use_file_metadata FILE_NAMES_AND_N_ISSUES = [ - ('case0_spaces.py', 3), + ('case0_spaces.py', 0), ('case1_simple_valid_program.py', 0), ('case2_boolean_expressions.py', 3), ('case3_redefining_builtin.py', 1), @@ -22,7 +22,7 @@ ('case15_redefining.py', 2), ('case16_comments.py', 0), ('case17_dangerous_default_value.py', 1), - ('case18_comprehensions.py', 2), + ('case18_comprehensions.py', 3), ('case19_bad_indentation.py', 2), ('case21_imports.py', 2), ('case23_merging_comparisons.py', 4), From 5f6406e1cc21a57adb32b41923ea38ac45f9927c Mon Sep 17 00:00:00 2001 From: Nastya Birillo Date: Fri, 23 Jul 2021 01:47:39 +0300 Subject: [PATCH 2/4] Add whitelist for flake8-spellcheck (#68) * Add whitelist for flake8-spellcheck * Add a test for spellcheck and ignore unnecessary rule * Fix flake8 config --- src/python/review/inspectors/flake8/flake8.py | 5 + .../review/inspectors/flake8/whitelist.txt | 127 ++++++++++++++++++ .../inspectors/test_flake8_inspector.py | 7 +- .../inspectors/python/case31_spellcheck.py | 3 + 4 files changed, 139 insertions(+), 3 deletions(-) create mode 100644 src/python/review/inspectors/flake8/whitelist.txt create mode 100644 test/resources/inspectors/python/case31_spellcheck.py diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index 6f071338..6ea27f09 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -13,6 +13,10 @@ logger = logging.getLogger(__name__) PATH_FLAKE8_CONFIG = Path(__file__).parent / '.flake8' +# To make the whitelist, a list of words was examined based on students' solutions +# that were flagged by flake8-spellcheck as erroneous. In general, the whitelist included those words +# that belonged to library methods and which were common abbreviations. +PATH_FLAKE8_SPELLCHECK_WHITELIST = Path(__file__).parent / 'whitelist.txt' FORMAT = '%(path)s:%(row)d:%(col)d:%(code)s:%(text)s' INSPECTOR_NAME = 'flake8' @@ -26,6 +30,7 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: 'flake8', f'--format={FORMAT}', f'--config={PATH_FLAKE8_CONFIG}', + f'--whitelist={PATH_FLAKE8_SPELLCHECK_WHITELIST}', '--max-complexity', '0', path ] diff --git a/src/python/review/inspectors/flake8/whitelist.txt b/src/python/review/inspectors/flake8/whitelist.txt new file mode 100644 index 00000000..8ab08f11 --- /dev/null +++ b/src/python/review/inspectors/flake8/whitelist.txt @@ -0,0 +1,127 @@ +aggfunc +appendleft +argmax +asctime +astype +betavariate +birthdate +blackbox +bs4 +byteorder +calc +capwords +casefold +caseless +concat +consts +coord +copysign +csgraph +ctime +dataframe +dataframes +dataset +datasets +decrypted +dedent +deque +desc +devs +df +dicts +dirs +divmod +dtype +edu +eig +elems +etree +expm1 +falsy +fillna +floordiv +fromstring +fullmatch +gensim +gmtime +groupby +halfs +hashable +href +hyp +hyperskill +iadd +iloc +inplace +ints +isalnum +isalpha +isin +islice +islower +isnumeric +isprintable +istitle +isub +iterrows +kcal +kcals +lastname +lemmatize +lemmatizer +lifes +lim +linalg +linspace +lowercased +lvl +lxml +matmul +multiline +ndarray +ndigits +ndim +nltk +nrows +numpy +nums +ost +param +params +parsers +pathlib +popleft +pos +punct +readline +rfind +rindex +rmdir +schur +scipy +sigmoid +sqrt +src +stemmer +stepik +subdicts +subdir +subdirs +substr +substring +textwrap +todos +tokenize +tokenized +tokenizer +tolist +tracklist +truediv +truthy +unpickled +upd +util +utils +webpage +whitespaces +writeback \ No newline at end of file diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 56e02aec..3c655331 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -14,7 +14,7 @@ ('case3_redefining_builtin.py', 1), ('case4_naming.py', 10), ('case5_returns.py', 1), - # ('case6_unused_variables.py', 3), + ('case6_unused_variables.py', 3), ('case8_good_class.py', 0), ('case7_empty_lines.py', 0), ('case10_unused_variable_in_loop.py', 1), @@ -28,6 +28,7 @@ ('case19_bad_indentation.py', 3), ('case21_imports.py', 2), ('case25_django.py', 0), + ('case31_spellcheck.py', 0), ] @@ -50,8 +51,8 @@ def test_file_with_issues(file_name: str, n_issues: int): n_cc=8)), ('case3_redefining_builtin.py', IssuesTestInfo(n_error_prone=1)), ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_best_practices=0, n_cc=5)), - # ('case6_unused_variables.py', IssuesTestInfo(n_best_practices=3, - # n_cc=1)), + ('case6_unused_variables.py', IssuesTestInfo(n_best_practices=3, + n_cc=1)), ('case8_good_class.py', IssuesTestInfo(n_cc=1)), ('case7_empty_lines.py', IssuesTestInfo(n_cc=4)), ('case10_unused_variable_in_loop.py', IssuesTestInfo(n_best_practices=1, diff --git a/test/resources/inspectors/python/case31_spellcheck.py b/test/resources/inspectors/python/case31_spellcheck.py new file mode 100644 index 00000000..b777eb79 --- /dev/null +++ b/test/resources/inspectors/python/case31_spellcheck.py @@ -0,0 +1,3 @@ +import math + +number = math.sqrt(float(input())) From 9a6d53cd1ca220d97d296c0087056b5885b26281 Mon Sep 17 00:00:00 2001 From: Nikolay Vyahhi Date: Mon, 26 Jul 2021 16:02:17 +0300 Subject: [PATCH 3/4] Bump version 1.1.0 -> 1.1.1 --- VERSION.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/VERSION.md b/VERSION.md index 9084fa2f..524cb552 100644 --- a/VERSION.md +++ b/VERSION.md @@ -1 +1 @@ -1.1.0 +1.1.1 From 07b90987bb613dc983fadcb4f539feca68f99b92 Mon Sep 17 00:00:00 2001 From: Nastya Birillo Date: Fri, 13 Aug 2021 18:07:59 +0300 Subject: [PATCH 4/4] Add Python inspections and a script for tool evaluation (#26) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * wps-light support (#17) Add wps-light support * New flake8 plugins support (#18) Added support for flake8-broken-line, flake8-string-format, flake8-commas (they are WPS dependencies). Added support for cohesion. * Radon support (#19) Added new inspector: Radon. Added maintainability index. * Requirements upgrade (#21) Updated versions of dependencies Added django dictionary support Fixed tests * xlsx-run-tool (#24) Added xlsx_tool_run.py – script to run the tool on multiple code samples stored in xlsx file * Update version to 1.2.0 * Rename output_format to format * Fix double quotes * Fix small issues * Fix small issues * Fix double quotes * Fix double quotes * Inspectors fix (#39) * Add origin class for maintainability index * Add WPS518 to ignore due to collision with C0200 by Pylint * New category (#38) * Added a new category INFO, in order not to take into account some issues in the evaluation. * Fixed trailing commas * Delete create_directory function * Fix GitHub actions (#44) * Use the same environment as in the production dockerfile * Fix Dockerfiles * Install some dependencies for TeamCity * Update build * Add checks from teamcity * Update build * Fix indentation * Fix PR comments * Delete init files from resources * Add init in multi file project * Fix flake8 tests * Main upd bugs fix (#74) * Delete evaluation part * Fix bugs with detekt, pmd and flake8 * Issues fix (#80) * Fixed issue #70 * Fixed issue #72: now only spellcheck checks brackets * Fixed issue #73 * Fixed issue #77 * Fixed issue #78 * Fixed issue #71 * Fixed issue #79 * Fixed issue #81 * Added case 36: unpacking * Added case 37: wildcard import * Ignoring WPS347 and F405 * Added cases 36 (unpacking) and 37 (wildcard_import). Also removed W0622 (redefining_builtin) * Removed W0622 (redefining_builtin) * Issue #87 fix (#90) * Fixed issue #87 * Added test * Fixed issues #91 and #92 (#97) * Delete xlsx * Remove openpyxl * Issues fix (#104) * Fixed #100 * Fixed #102 * Added some more new words * Added toplevel * Update flake8 whitelist * Recovered accidentally deleted words Co-authored-by: Vlasov Ilya <55441714+GirZ0n@users.noreply.github.com> Co-authored-by: Daria Diatlova Co-authored-by: Nikolay Vyahhi Co-authored-by: Ilya Vlasov --- .github/workflows/build.yml | 18 ++- README.md | 9 +- VERSION.md | 2 +- requirements-test.txt | 6 +- requirements.txt | 6 + setup.py | 26 +-- .../python => src/python/common}/__init__.py | 0 src/python/common/tool_arguments.py | 74 +++++++++ src/python/review/application_config.py | 26 ++- src/python/review/common/file_system.py | 15 +- src/python/review/common/java_compiler.py | 2 +- src/python/review/common/parallel_runner.py | 2 +- src/python/review/common/subprocess_runner.py | 2 +- .../inspectors/checkstyle/checkstyle.py | 4 +- .../inspectors/checkstyle/files/config.xml | 47 ++++-- src/python/review/inspectors/common.py | 23 +++ src/python/review/inspectors/detekt/detekt.py | 4 +- .../review/inspectors/detekt/issue_types.py | 2 +- src/python/review/inspectors/eslint/eslint.py | 2 +- src/python/review/inspectors/flake8/.flake8 | 47 ++++++ src/python/review/inspectors/flake8/flake8.py | 44 +++-- .../review/inspectors/flake8/issue_types.py | 87 ++++++++++ .../review/inspectors/flake8/whitelist.txt | 22 ++- .../review/inspectors/inspector_type.py | 2 + .../review/inspectors/intellij/intellij.py | 8 +- .../intellij/issue_types/__init__.py | 19 ++- .../inspectors/intellij/issue_types/kotlin.py | 2 +- src/python/review/inspectors/issue.py | 14 +- .../inspectors/parsers/checkstyle_parser.py | 22 ++- .../review/inspectors/pmd/files/bin/basic.xml | 21 ++- .../review/inspectors/pmd/issue_types.py | 2 +- src/python/review/inspectors/pmd/pmd.py | 44 ++++- .../review/inspectors/pyast/python_ast.py | 14 +- .../review/inspectors/pylint/issue_types.py | 2 + src/python/review/inspectors/pylint/pylint.py | 6 +- src/python/review/inspectors/pylint/pylintrc | 2 + .../review/inspectors/radon}/__init__.py | 0 src/python/review/inspectors/radon/radon.py | 56 +++++++ .../review/inspectors/spotbugs/spotbugs.py | 4 +- .../inspectors/springlint/springlint.py | 39 +++-- src/python/review/inspectors/tips.py | 71 +++++--- src/python/review/logging_config.py | 12 +- src/python/review/quality/evaluate_quality.py | 51 ++++-- src/python/review/quality/model.py | 12 +- .../quality/rules/best_practices_scoring.py | 4 +- .../quality/rules/boolean_length_scoring.py | 6 +- .../quality/rules/class_response_scoring.py | 4 +- .../quality/rules/code_style_scoring.py | 8 +- .../review/quality/rules/cohesion_scoring.py | 71 ++++++++ .../review/quality/rules/coupling_scoring.py | 4 +- .../rules/cyclomatic_complexity_scoring.py | 10 +- .../quality/rules/error_prone_scoring.py | 2 +- .../quality/rules/function_length_scoring.py | 12 +- .../rules/inheritance_depth_scoring.py | 2 +- .../review/quality/rules/line_len_scoring.py | 4 +- .../quality/rules/maintainability_scoring.py | 74 +++++++++ .../quality/rules/method_number_scoring.py | 4 +- .../quality/rules/weighted_methods_scoring.py | 4 +- src/python/review/reviewers/common.py | 8 +- src/python/review/reviewers/perform_review.py | 4 +- src/python/review/reviewers/python.py | 2 +- .../review/reviewers/utils/code_statistics.py | 16 +- .../review/reviewers/utils/issues_filter.py | 10 +- .../review/reviewers/utils/print_review.py | 6 +- src/python/review/run_tool.py | 97 +++++------ test/python/functional_tests/conftest.py | 5 +- .../test_different_languages.py | 9 +- test/python/functional_tests/test_disable.py | 5 +- .../functional_tests/test_duplicates.py | 5 +- .../python/functional_tests/test_exit_code.py | 7 +- .../functional_tests/test_file_or_project.py | 5 +- .../test_multi_file_project.py | 27 ++-- .../functional_tests/test_range_of_lines.py | 31 ++-- .../test_single_file_json_format.py | 19 ++- .../python/functional_tests/test_verbosity.py | 7 +- test/python/inspectors/conftest.py | 25 +-- .../inspectors/test_checkstyle_inspector.py | 7 +- .../inspectors/test_detekt_inspector.py | 7 +- .../inspectors/test_eslint_inspector.py | 6 +- .../inspectors/test_flake8_inspector.py | 63 +++++--- test/python/inspectors/test_local_review.py | 7 +- .../inspectors/test_out_of_range_issues.py | 2 +- test/python/inspectors/test_pmd_inspector.py | 8 +- .../inspectors/test_pylint_inspector.py | 19 ++- test/python/inspectors/test_python_ast.py | 12 +- .../python/inspectors/test_radon_inspector.py | 52 ++++++ .../inspectors/test_springlint_inspector.py | 3 +- .../java/test_multiple_literals.java | 32 ++++ .../inspectors/python/case13_complex_logic.py | 82 ++++------ .../python/case13_complex_logic_2.py | 16 +- .../python/case18_comprehensions.py | 2 +- .../inspectors/python/case32_string_format.py | 143 ++++++++++++++++ .../inspectors/python/case33_commas.py | 153 ++++++++++++++++++ .../inspectors/python/case34_cohesion.py | 28 ++++ .../inspectors/python/case35_line_break.py | 54 +++++++ .../inspectors/python/case36_unpacking.py | 10 ++ .../python/case37_wildcard_import.py | 33 ++++ .../python/case3_redefining_builtin.py | 5 +- .../inspectors/python/case4_naming.py | 2 +- .../inspectors/python/case7_empty_lines.py | 5 +- .../inspectors/python_ast/__init__.py | 0 whitelist.txt | 35 ++++ 102 files changed, 1706 insertions(+), 450 deletions(-) rename {test/resources/functional_tests/different_languages/python => src/python/common}/__init__.py (100%) create mode 100644 src/python/common/tool_arguments.py create mode 100644 src/python/review/inspectors/common.py rename {test/resources/inspectors/python => src/python/review/inspectors/radon}/__init__.py (100%) create mode 100644 src/python/review/inspectors/radon/radon.py create mode 100644 src/python/review/quality/rules/cohesion_scoring.py create mode 100644 src/python/review/quality/rules/maintainability_scoring.py create mode 100644 test/python/inspectors/test_radon_inspector.py create mode 100644 test/resources/inspectors/java/test_multiple_literals.java create mode 100644 test/resources/inspectors/python/case32_string_format.py create mode 100644 test/resources/inspectors/python/case33_commas.py create mode 100644 test/resources/inspectors/python/case34_cohesion.py create mode 100644 test/resources/inspectors/python/case35_line_break.py create mode 100644 test/resources/inspectors/python/case36_unpacking.py create mode 100644 test/resources/inspectors/python/case37_wildcard_import.py delete mode 100644 test/resources/inspectors/python_ast/__init__.py diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 54fe8f08..912f8a92 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -22,7 +22,7 @@ jobs: # stop the build if there are Python syntax errors or undefined names flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules # TODO: change max-complexity into 10 after refactoring - flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=I201,I202,I101,I100,R504,A003,E800,SC200,SC100,E402 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules + flake8 . --count --max-complexity=11 --max-line-length=120 --max-doc-length=120 --ignore=R504,A003,E800,E402,W503,WPS,H601 --statistics --exclude=.git,__pycache__,docs/source/conf.py,old,build,dist,venv,test/resources,.eggs,review.egg-info,.pytest_cache,node_modules - name: Set up Eslint run: | @@ -32,3 +32,19 @@ jobs: - name: Test with pytest run: | pytest + + - name: Install review module + run: | + pip install . + + - name: Check installed module can run python linters + run: | + review setup.py + + - name: Check installed module can run java linters + run: | + review test/resources/inspectors/java/test_algorithm_with_scanner.java + + - name: Check installed module can run js linters + run: | + review test/resources/inspectors/js/case0_no_issues.js diff --git a/README.md b/README.md index 4f9856dc..24db74c0 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,10 @@ Python language: - [x] Pylint [GNU LGPL v2] * [Site and docs](https://www.pylint.org/) * [Repository](https://github.com/PyCQA/pylint) - + +- [x] Radon [MIT] + * [Site and docs](https://radon.readthedocs.io/en/latest/) + * [Repository](https://github.com/rubik/radon) Java language: @@ -92,7 +95,7 @@ Argument | Description --- | --- **‑h**, **‑‑help** | show the help message and exit. **‑v**, **‑‑verbosity** | choose logging level according [this](https://docs.python.org/3/library/logging.html#levels) list: `1` - **ERROR**; `2` - **INFO**; `3` - **DEBUG**; `0` - disable logging (**CRITICAL** value); default value is `0` (**CRITICAL**). -**‑d**, **‑‑disable** | disable inspectors. Available values: for **Python** language: `pylint` for [Pylint](https://github.com/PyCQA/pylint), `flake8` for [flake8](https://flake8.pycqa.org/en/latest/), `python_ast` to check different measures providing by AST; for **Java** language: `checkstyle` for the [Checkstyle](https://checkstyle.sourceforge.io/), `pmd` for [PMD](https://pmd.github.io/); for `Kotlin` language: detekt for [Detekt](https://detekt.github.io/detekt/); for **JavaScript** language: `eslint` for [ESlint](https://eslint.org/). Example: `-d pylint,flake8`. +**‑d**, **‑‑disable** | disable inspectors. Available values: for **Python** language: `pylint` for [Pylint](https://github.com/PyCQA/pylint), `flake8` for [flake8](https://flake8.pycqa.org/en/latest/), `radon` for [Radon](https://radon.readthedocs.io/en/latest/), `python_ast` to check different measures providing by AST; for **Java** language: `checkstyle` for the [Checkstyle](https://checkstyle.sourceforge.io/), `pmd` for [PMD](https://pmd.github.io/); for `Kotlin` language: detekt for [Detekt](https://detekt.github.io/detekt/); for **JavaScript** language: `eslint` for [ESlint](https://eslint.org/). Example: `-d pylint,flake8`. **‑‑allow-duplicates** | allow duplicate issues found by different linters. By default, duplicates are skipped. **‑‑language-version**, **‑‑language_version** | specify the language version for JAVA inspectors. Available values: `java7`, `java8`, `java9`, `java11`. **Note**: **‑‑language_version** is deprecated. Will be deleted in the future. **‑‑n-cpu**, **‑‑n_cpu** | specify number of _cpu_ that can be used to run inspectors. **Note**: **‑‑n_cpu** is deprecated. Will be deleted in the future. @@ -193,7 +196,7 @@ __Note__: If you have `ModuleNotFoundError` while you try to run tests, please c __Note__: We use [eslint](https://eslint.org/) and [open-jdk 11](https://openjdk.java.net/projects/jdk/11/) in the tests. Please, set up the environment before running the tests. -You can see en example of the environment configuration in +You can see en example of the environment configuration in the [Dockerfile](Dockerfile) file. Use `pytest` from the root directory to run __ALL__ tests. diff --git a/VERSION.md b/VERSION.md index 524cb552..26aaba0e 100644 --- a/VERSION.md +++ b/VERSION.md @@ -1 +1 @@ -1.1.1 +1.2.0 diff --git a/requirements-test.txt b/requirements-test.txt index c2bf26e0..cc876366 100644 --- a/requirements-test.txt +++ b/requirements-test.txt @@ -2,4 +2,8 @@ pytest==6.2.3 pytest-runner==5.2 pytest-subtests==0.4.0 jsonschema==3.2.0 -pandas==1.2.3 \ No newline at end of file +pandas==1.2.3 +django==3.2 +pylint==2.7.4 +requests==2.25.1 +setuptools==56.0.0 \ No newline at end of file diff --git a/requirements.txt b/requirements.txt index a859e972..484495a5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -17,6 +17,12 @@ flake8-return==1.1.2 flake8-spellcheck==0.24.0 mccabe==0.6.1 pep8-naming==0.11.1 +wps-light==0.15.2 +flake8-broken-line==0.3.0 +flake8-string-format==0.3.0 +flake8-commas==2.0.0 +cohesion==1.0.0 +radon==4.5.0 # extra libraries and frameworks django==3.2 diff --git a/setup.py b/setup.py index 2e1e7717..8b23d17a 100644 --- a/setup.py +++ b/setup.py @@ -19,13 +19,11 @@ def get_version() -> str: def get_inspectors_additional_files() -> List[str]: inspectors_path = current_dir / 'src' / 'python' / 'review' / 'inspectors' - result = [] for root, _, files in os.walk(inspectors_path): for file in files: - file_path = Path(root) / file - if not file_path.name.endswith('.py'): - result.append(str(file_path)) + if not file.endswith('.py'): + result.append(str(Path(root) / file)) return result @@ -45,22 +43,28 @@ def get_inspectors_additional_files() -> List[str]: 'Topic :: Software Development :: Build Tools', 'License :: OSI Approved :: MIT License', 'Programming Language :: Python :: 3', - 'Operating System :: OS Independent' + 'Operating System :: OS Independent', ], keywords='code review', python_requires='>=3.8, <4', install_requires=['upsourceapi'], packages=find_packages(exclude=[ - '*.unit_tests', '*.unit_tests.*', 'unit_tests.*', 'unit_tests', - '*.functional_tests', '*.functional_tests.*', 'functional_tests.*', 'functional_tests' + '*.unit_tests', + '*.unit_tests.*', + 'unit_tests.*', + 'unit_tests', + '*.functional_tests', + '*.functional_tests.*', + 'functional_tests.*', + 'functional_tests', ]), zip_safe=False, package_data={ - '': get_inspectors_additional_files() + '': get_inspectors_additional_files(), }, entry_points={ 'console_scripts': [ - 'review=src.python.review.run_tool:main' - ] - } + 'review=src.python.review.run_tool:main', + ], + }, ) diff --git a/test/resources/functional_tests/different_languages/python/__init__.py b/src/python/common/__init__.py similarity index 100% rename from test/resources/functional_tests/different_languages/python/__init__.py rename to src/python/common/__init__.py diff --git a/src/python/common/tool_arguments.py b/src/python/common/tool_arguments.py new file mode 100644 index 00000000..db0c77da --- /dev/null +++ b/src/python/common/tool_arguments.py @@ -0,0 +1,74 @@ +from dataclasses import dataclass +from enum import Enum, unique +from typing import List, Optional + +from src.python.review.application_config import LanguageVersion +from src.python.review.inspectors.inspector_type import InspectorType + + +@unique +class VerbosityLevel(Enum): + """ + Same meaning as the logging level. Should be used in command-line args. + """ + DEBUG = 3 + INFO = 2 + ERROR = 1 + DISABLE = 0 + + @classmethod + def values(cls) -> List[int]: + return [member.value for member in VerbosityLevel] + + +@dataclass(frozen=True) +class ArgumentsInfo: + short_name: Optional[str] + long_name: str + description: str + + +@unique +class RunToolArgument(Enum): + VERBOSITY = ArgumentsInfo('-v', '--verbosity', + 'Choose logging level: ' + f'{VerbosityLevel.ERROR.value} - ERROR; ' + f'{VerbosityLevel.INFO.value} - INFO; ' + f'{VerbosityLevel.DEBUG.value} - DEBUG; ' + f'{VerbosityLevel.DISABLE.value} - disable logging; ' + 'default is 0') + + inspectors = [inspector.lower() for inspector in InspectorType.available_values()] + disabled_inspectors_example = f'-d {inspectors[0].lower()},{inspectors[1].lower()}' + + DISABLE = ArgumentsInfo('-d', '--disable', + 'Disable inspectors. ' + f'Available values: {", ".join(inspectors)}. ' + f'Example: {disabled_inspectors_example}') + + DUPLICATES = ArgumentsInfo(None, '--allow-duplicates', + 'Allow duplicate issues found by different linters. ' + 'By default, duplicates are skipped.') + + LANG_VERSION = ArgumentsInfo(None, '--language-version', + 'Specify the language version for JAVA inspectors.' + 'Available values are: ' + f'{LanguageVersion.PYTHON_3.value}, {LanguageVersion.JAVA_8.value}, ' + f'{LanguageVersion.JAVA_11.value}, {LanguageVersion.KOTLIN.value}.') + + CPU = ArgumentsInfo(None, '--n-cpu', + 'Specify number of cpu that can be used to run inspectors') + + PATH = ArgumentsInfo(None, 'path', 'Path to file or directory to inspect.') + + FORMAT = ArgumentsInfo('-f', '--format', + 'The output format. Default is JSON.') + + START_LINE = ArgumentsInfo('-s', '--start-line', + 'The first line to be analyzed. It starts from 1.') + + END_LINE = ArgumentsInfo('-e', '--end-line', 'The end line to be analyzed or None.') + + NEW_FORMAT = ArgumentsInfo(None, '--new-format', + 'The argument determines whether the tool ' + 'should use the new format') diff --git a/src/python/review/application_config.py b/src/python/review/application_config.py index c678605f..6032aef3 100644 --- a/src/python/review/application_config.py +++ b/src/python/review/application_config.py @@ -1,7 +1,8 @@ from dataclasses import dataclass from enum import Enum, unique -from typing import Optional, Set, List +from typing import List, Optional, Set +from src.python.review.common.file_system import Extension from src.python.review.inspectors.inspector_type import InspectorType @@ -22,7 +23,30 @@ class LanguageVersion(Enum): JAVA_8 = 'java8' JAVA_9 = 'java9' JAVA_11 = 'java11' + PYTHON_3 = 'python3' + KOTLIN = 'kotlin' @classmethod def values(cls) -> List[str]: return [member.value for member in cls.__members__.values()] + + @classmethod + def language_to_extension_dict(cls) -> dict: + return {cls.PYTHON_3.value: Extension.PY.value, + cls.JAVA_7.value: Extension.JAVA.value, + cls.JAVA_8.value: Extension.JAVA.value, + cls.JAVA_9.value: Extension.JAVA.value, + cls.JAVA_11.value: Extension.JAVA.value, + cls.KOTLIN.value: Extension.KT.value} + + @classmethod + def language_by_extension(cls, lang: str) -> str: + return cls.language_to_extension_dict()[lang] + + def is_java(self) -> bool: + return ( + self == LanguageVersion.JAVA_7 + or self == LanguageVersion.JAVA_8 + or self == LanguageVersion.JAVA_9 + or self == LanguageVersion.JAVA_11 + ) diff --git a/src/python/review/common/file_system.py b/src/python/review/common/file_system.py index 6decb410..5c53d9d4 100644 --- a/src/python/review/common/file_system.py +++ b/src/python/review/common/file_system.py @@ -4,7 +4,7 @@ from contextlib import contextmanager from enum import Enum, unique from pathlib import Path -from typing import List, Union, Callable +from typing import Callable, List, Union @unique @@ -65,19 +65,16 @@ def new_temp_dir() -> Path: def create_file(file_path: Union[str, Path], content: str): file_path = Path(file_path) - create_directory(os.path.dirname(file_path)) - with open(file_path, 'w') as f: - f.write(content) - - -def create_directory(directory: str) -> None: - os.makedirs(directory, exist_ok=True) + os.makedirs(os.path.dirname(file_path), exist_ok=True) + with open(file_path, 'w+') as f: + f.writelines(content) + yield Path(file_path) def get_file_line(path: Path, line_number: int): return linecache.getline( str(path), - line_number + line_number, ).strip() diff --git a/src/python/review/common/java_compiler.py b/src/python/review/common/java_compiler.py index aee76032..e6b97861 100644 --- a/src/python/review/common/java_compiler.py +++ b/src/python/review/common/java_compiler.py @@ -18,7 +18,7 @@ def javac(javac_args: Union[str, Path]) -> bool: output_bytes: bytes = subprocess.check_output( f'javac {javac_args}', shell=True, - stderr=subprocess.STDOUT + stderr=subprocess.STDOUT, ) output_str = str(output_bytes, Encoding.UTF_ENCODING.value) diff --git a/src/python/review/common/parallel_runner.py b/src/python/review/common/parallel_runner.py index e9ab7d8d..c0347d23 100644 --- a/src/python/review/common/parallel_runner.py +++ b/src/python/review/common/parallel_runner.py @@ -37,7 +37,7 @@ def inspect_in_parallel(path: Path, with multiprocessing.Pool(config.n_cpu) as pool: issues = pool.map( functools.partial(run_inspector, path, config), - inspectors + inspectors, ) return list(itertools.chain(*issues)) diff --git a/src/python/review/common/subprocess_runner.py b/src/python/review/common/subprocess_runner.py index 3dd5cad3..a25cbdcd 100644 --- a/src/python/review/common/subprocess_runner.py +++ b/src/python/review/common/subprocess_runner.py @@ -9,7 +9,7 @@ def run_in_subprocess(command: List[str]) -> str: process = subprocess.run( command, stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout = process.stdout.decode() diff --git a/src/python/review/inspectors/checkstyle/checkstyle.py b/src/python/review/inspectors/checkstyle/checkstyle.py index 389d889f..c796fbf9 100644 --- a/src/python/review/inspectors/checkstyle/checkstyle.py +++ b/src/python/review/inspectors/checkstyle/checkstyle.py @@ -35,7 +35,7 @@ class CheckstyleInspector(BaseInspector): r'Boolean expression complexity is (\d+)', 'LineLengthCheck': - r'Line is longer than \d+ characters \(found (\d+)\)' + r'Line is longer than \d+ characters \(found (\d+)\)', } @classmethod @@ -43,7 +43,7 @@ def _create_command(cls, path: Path, output_path: Path) -> List[str]: return [ 'java', '-jar', PATH_TOOLS_CHECKSTYLE_JAR, '-c', PATH_TOOLS_CHECKSTYLE_CONFIG, - '-f', 'xml', '-o', output_path, str(path) + '-f', 'xml', '-o', output_path, str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: diff --git a/src/python/review/inspectors/checkstyle/files/config.xml b/src/python/review/inspectors/checkstyle/files/config.xml index 00222e43..81fbc370 100644 --- a/src/python/review/inspectors/checkstyle/files/config.xml +++ b/src/python/review/inspectors/checkstyle/files/config.xml @@ -44,8 +44,9 @@ - - + + + @@ -87,16 +88,42 @@ + + + + + + + + + + + + + + + + - - - - - - - - + + + + + + + + + diff --git a/src/python/review/inspectors/common.py b/src/python/review/inspectors/common.py new file mode 100644 index 00000000..a307bd96 --- /dev/null +++ b/src/python/review/inspectors/common.py @@ -0,0 +1,23 @@ +from math import floor + + +def convert_percentage_of_value_to_lack_of_value(percentage_of_value: float) -> int: + """ + Converts percentage of value to lack of value. + Calculated by the formula: floor(100 - percentage_of_value). + + :param percentage_of_value: value in the range from 0 to 100. + :return: lack of value. + """ + return floor(100 - percentage_of_value) + + +# TODO: When upgrading to python 3.9+, replace it with removeprefix. +# See: https://docs.python.org/3.9/library/stdtypes.html#str.removeprefix +def remove_prefix(text: str, prefix: str) -> str: + """ + Removes the prefix if it is present, otherwise returns the original string. + """ + if text.startswith(prefix): + return text[len(prefix):] + return text diff --git a/src/python/review/inspectors/detekt/detekt.py b/src/python/review/inspectors/detekt/detekt.py index 741a334d..041c6b2d 100644 --- a/src/python/review/inspectors/detekt/detekt.py +++ b/src/python/review/inspectors/detekt/detekt.py @@ -27,7 +27,7 @@ class DetektInspector(BaseInspector): 'ComplexCondition': r'This condition is too complex \((\d+)\)', 'ComplexMethod': - r'The function .* appears to be too complex \((\d+)\)' + r'The function .* appears to be too complex \((\d+)\)', } @classmethod @@ -38,7 +38,7 @@ def _create_command(cls, path: Path, output_path: Path): '--config', PATH_DETEKT_CONFIG, '--plugins', PATH_DETEKT_PLUGIN, '--report', f'xml:{output_path}', - '--input', str(path) + '--input', str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: diff --git a/src/python/review/inspectors/detekt/issue_types.py b/src/python/review/inspectors/detekt/issue_types.py index e05d1a30..2379bba8 100644 --- a/src/python/review/inspectors/detekt/issue_types.py +++ b/src/python/review/inspectors/detekt/issue_types.py @@ -15,7 +15,7 @@ # complexity: 'ComplexCondition': IssueType.BOOL_EXPR_LEN, 'ComplexInterface': IssueType.COMPLEXITY, - 'ComplexMethod': IssueType.COMPLEXITY, + 'ComplexMethod': IssueType.CYCLOMATIC_COMPLEXITY, 'LabeledExpression': IssueType.COMPLEXITY, 'LargeClass': IssueType.COMPLEXITY, 'LongMethod': IssueType.FUNC_LEN, diff --git a/src/python/review/inspectors/eslint/eslint.py b/src/python/review/inspectors/eslint/eslint.py index 19d801cf..c7d0b1b4 100644 --- a/src/python/review/inspectors/eslint/eslint.py +++ b/src/python/review/inspectors/eslint/eslint.py @@ -17,7 +17,7 @@ class ESLintInspector(BaseInspector): origin_class_to_pattern = { 'complexity': - r'complexity of (\d+)' + r'complexity of (\d+)', } @classmethod diff --git a/src/python/review/inspectors/flake8/.flake8 b/src/python/review/inspectors/flake8/.flake8 index 6270ce48..f0452e5a 100644 --- a/src/python/review/inspectors/flake8/.flake8 +++ b/src/python/review/inspectors/flake8/.flake8 @@ -1,5 +1,8 @@ [flake8] disable_noqa=True + +dictionaries=en_US,python,technical,django + ignore=W291, # trailing whitespaces W292, # no newline at end of file W293, # blank line contains whitespaces @@ -13,3 +16,47 @@ ignore=W291, # trailing whitespaces E301, E302, E303, E304, E305, # problem with stepik templates E402, # module level import not at top of file I100, # Import statements are in the wrong order + # WPS: Naming + WPS110, # Forbid blacklisted variable names. + WPS111, # Forbid short variable or module names. + WPS112, # Forbid private name pattern. + WPS114, # Forbid names with underscored numbers pattern. + WPS125, # Forbid variable or module names which shadow builtin names. TODO: Collision with flake8-builtins + # WPS: Consistency + WPS303, # Forbid underscores in numbers. + WPS305, # Forbid f strings. + WPS306, # Forbid writing classes without base classes. + WPS317, # Forbid incorrect indentation for parameters. (Because of the unnecessary strictness) + WPS318, # Forbid extra indentation. TODO: Collision with standard flake8 indentation check + WPS319, # Forbid brackets in the wrong position. (Because of the unnecessary strictness) + WPS323, # Forbid % formatting on strings. + WPS324, # Enforce consistent return statements. TODO: Collision with flake8-return + WPS335, # Forbid wrong for loop iter targets. + WPS347, # Forbid imports that may cause confusion outside of the module. (controversial) + WPS358, # Forbid using float zeros: 0.0. + WPS359, # Forbids to unpack iterable objects to lists. (Because of its similarity to "WPS414") + WPS362, # Forbid assignment to a subscript slice. + # WPS: Best practices + WPS404, # Forbid complex defaults. TODO: Collision with "B006" + WPS420, # Forbid some python keywords. + WPS421, # Forbid calling some built-in functions.(e.g., print) + WPS429, # Forbid multiple assignments on the same line. + WPS430, # Forbid nested functions. + WPS431, # Forbid nested classes. + WPS435, # Forbid multiplying lists. + # WPS: Refactoring + WPS518, # Forbid implicit enumerate() calls. TODO: Collision with "C0200" + WPS527, # Require tuples as arguments for frozenset. + # WPS: OOP + WPS602, # Forbid @staticmethod decorator. + # flake8-string-format + P101, # format string does contain unindexed parameters + P102, # docstring does contain unindexed parameters + P103, # other string does contain unindexed parameters + F405, # Name may be undefined, or defined from star imports (Collision with the stricter "F403") + F522, # unused named arguments. TODO: Collision with "P302" + F523, # unused positional arguments. TODO: Collision with "P301" + F524, # missing argument. TODO: Collision with "P201" and "P202" + F525, # mixing automatic and manual numbering. TODO: Collision with "P205" + # flake8-commas + C814, # missing trailing comma in Python 2 diff --git a/src/python/review/inspectors/flake8/flake8.py b/src/python/review/inspectors/flake8/flake8.py index 6ea27f09..c5dfd274 100644 --- a/src/python/review/inspectors/flake8/flake8.py +++ b/src/python/review/inspectors/flake8/flake8.py @@ -5,9 +5,17 @@ from src.python.review.common.subprocess_runner import run_in_subprocess from src.python.review.inspectors.base_inspector import BaseInspector +from src.python.review.inspectors.common import convert_percentage_of_value_to_lack_of_value from src.python.review.inspectors.flake8.issue_types import CODE_PREFIX_TO_ISSUE_TYPE, CODE_TO_ISSUE_TYPE from src.python.review.inspectors.inspector_type import InspectorType -from src.python.review.inspectors.issue import BaseIssue, CodeIssue, CyclomaticComplexityIssue, IssueType, IssueData +from src.python.review.inspectors.issue import ( + BaseIssue, + CodeIssue, + CohesionIssue, + CyclomaticComplexityIssue, + IssueData, + IssueType, +) from src.python.review.inspectors.tips import get_cyclomatic_complexity_tip logger = logging.getLogger(__name__) @@ -32,7 +40,8 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: f'--config={PATH_FLAKE8_CONFIG}', f'--whitelist={PATH_FLAKE8_SPELLCHECK_WHITELIST}', '--max-complexity', '0', - path + '--cohesion-below', '100', + path, ] output = run_in_subprocess(command) return cls.parse(output) @@ -41,12 +50,14 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: def parse(cls, output: str) -> List[BaseIssue]: row_re = re.compile(r'^(.*):(\d+):(\d+):([A-Z]+\d{3}):(.*)$', re.M) cc_description_re = re.compile(r"'(.+)' is too complex \((\d+)\)") + cohesion_description_re = re.compile(r"class has low \((\d*\.?\d*)%\) cohesion") issues: List[BaseIssue] = [] for groups in row_re.findall(output): description = groups[4] origin_class = groups[3] cc_match = cc_description_re.match(description) + cohesion_match = cohesion_description_re.match(description) file_path = Path(groups[0]) line_no = int(groups[1]) @@ -56,26 +67,39 @@ def parse(cls, output: str) -> List[BaseIssue]: line_number=line_no, column_number=column_number, origin_class=origin_class) - if cc_match is not None: - issue_data['description'] = get_cyclomatic_complexity_tip() - issue_data['cc_value'] = int(cc_match.groups()[1]) - issue_data['type'] = IssueType.CYCLOMATIC_COMPLEXITY + if cc_match is not None: # mccabe: cyclomatic complexity + issue_data[IssueData.DESCRIPTION.value] = get_cyclomatic_complexity_tip() + issue_data[IssueData.CYCLOMATIC_COMPLEXITY.value] = int(cc_match.groups()[1]) + issue_data[IssueData.ISSUE_TYPE.value] = IssueType.CYCLOMATIC_COMPLEXITY issues.append(CyclomaticComplexityIssue(**issue_data)) + elif cohesion_match is not None: # flake8-cohesion + issue_data[IssueData.DESCRIPTION.value] = description # TODO: Add tip + issue_data[IssueData.COHESION_LACK.value] = convert_percentage_of_value_to_lack_of_value( + float(cohesion_match.group(1)), + ) + issue_data[IssueData.ISSUE_TYPE.value] = IssueType.COHESION + issues.append(CohesionIssue(**issue_data)) else: issue_type = cls.choose_issue_type(origin_class) - issue_data['type'] = issue_type - issue_data['description'] = description + issue_data[IssueData.ISSUE_TYPE.value] = issue_type + issue_data[IssueData.DESCRIPTION.value] = description issues.append(CodeIssue(**issue_data)) return issues @staticmethod def choose_issue_type(code: str) -> IssueType: + # Handling individual codes if code in CODE_TO_ISSUE_TYPE: return CODE_TO_ISSUE_TYPE[code] - code_prefix = re.match(r'^([a-z]+)\d+$', code, re.IGNORECASE).group(1) - issue_type = CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix) + regex_match = re.match(r'^([A-Z]+)(\d)\d*$', code, re.IGNORECASE) + code_prefix = regex_match.group(1) + first_code_number = regex_match.group(2) + + # Handling other issues + issue_type = (CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix + first_code_number) + or CODE_PREFIX_TO_ISSUE_TYPE.get(code_prefix)) if not issue_type: logger.warning(f'flake8: {code} - unknown error code') return IssueType.BEST_PRACTICES diff --git a/src/python/review/inspectors/flake8/issue_types.py b/src/python/review/inspectors/flake8/issue_types.py index 7997d32c..b1074b52 100644 --- a/src/python/review/inspectors/flake8/issue_types.py +++ b/src/python/review/inspectors/flake8/issue_types.py @@ -15,12 +15,92 @@ # builtin naming 'A003': IssueType.BEST_PRACTICES, + + # flake8-broken-line + 'N400': IssueType.CODE_STYLE, + + # flake8-commas + 'C812': IssueType.CODE_STYLE, + 'C813': IssueType.CODE_STYLE, + 'C815': IssueType.CODE_STYLE, + 'C816': IssueType.CODE_STYLE, + 'C818': IssueType.CODE_STYLE, + 'C819': IssueType.CODE_STYLE, + + # The categorization for WPS was created using the following document: https://bit.ly/3yms06n + + # WPS: Naming + 'WPS117': IssueType.CODE_STYLE, # Forbid naming variables self, cls, or mcs. + 'WPS125': IssueType.ERROR_PRONE, # Forbid variable or module names which shadow builtin names. + + # WPS: Consistency + 'WPS300': IssueType.CODE_STYLE, # Forbid imports relative to the current folder. + 'WPS301': IssueType.CODE_STYLE, # Forbid imports like import os.path. + 'WPS304': IssueType.CODE_STYLE, # Forbid partial floats like .05 or 23.. + 'WPS310': IssueType.BEST_PRACTICES, # Forbid uppercase X, O, B, and E in numbers. + 'WPS313': IssueType.CODE_STYLE, # Enforce separation of parenthesis from keywords with spaces. + 'WPS317': IssueType.CODE_STYLE, # Forbid incorrect indentation for parameters. + 'WPS318': IssueType.CODE_STYLE, # Forbid extra indentation. + 'WPS319': IssueType.CODE_STYLE, # Forbid brackets in the wrong position. + 'WPS320': IssueType.CODE_STYLE, # Forbid multi-line function type annotations. + 'WPS321': IssueType.CODE_STYLE, # Forbid uppercase string modifiers. + 'WPS324': IssueType.ERROR_PRONE, # If any return has a value, all return nodes should have a value. + 'WPS325': IssueType.ERROR_PRONE, # If any yield has a value, all yield nodes should have a value. + 'WPS326': IssueType.ERROR_PRONE, # Forbid implicit string concatenation. + 'WPS329': IssueType.ERROR_PRONE, # Forbid meaningless except cases. + 'WPS330': IssueType.ERROR_PRONE, # Forbid unnecessary operators in your code. + 'WPS338': IssueType.BEST_PRACTICES, # Forbid incorrect order of methods inside a class. + 'WPS339': IssueType.CODE_STYLE, # Forbid meaningless zeros. + 'WPS340': IssueType.CODE_STYLE, # Forbid extra + signs in the exponent. + 'WPS341': IssueType.CODE_STYLE, # Forbid lowercase letters as hex numbers. + 'WPS343': IssueType.CODE_STYLE, # Forbid uppercase complex number suffix. + 'WPS344': IssueType.ERROR_PRONE, # Forbid explicit division (or modulo) by zero. + 'WPS347': IssueType.ERROR_PRONE, # Forbid imports that may cause confusion outside of the module. + 'WPS348': IssueType.CODE_STYLE, # Forbid starting lines with a dot. + 'WPS350': IssueType.CODE_STYLE, # Enforce using augmented assign pattern. + 'WPS355': IssueType.CODE_STYLE, # Forbid useless blank lines before and after brackets. + 'WPS361': IssueType.CODE_STYLE, # Forbids inconsistent newlines in comprehensions. + + # WPS: Best practices + 'WPS405': IssueType.ERROR_PRONE, # Forbid anything other than ast.Name to define loop variables. + 'WPS406': IssueType.ERROR_PRONE, # Forbid anything other than ast.Name to define contexts. + 'WPS408': IssueType.ERROR_PRONE, # Forbid using the same logical conditions in one expression. + 'WPS414': IssueType.ERROR_PRONE, # Forbid tuple unpacking with side-effects. + 'WPS415': IssueType.ERROR_PRONE, # Forbid the same exception class in multiple except blocks. + 'WPS416': IssueType.ERROR_PRONE, # Forbid yield keyword inside comprehensions. + 'WPS417': IssueType.ERROR_PRONE, # Forbid duplicate items in hashes. + 'WPS418': IssueType.ERROR_PRONE, # Forbid exceptions inherited from BaseException. + 'WPS419': IssueType.ERROR_PRONE, # Forbid multiple returning paths with try / except case. + 'WPS424': IssueType.ERROR_PRONE, # Forbid BaseException exception. + 'WPS426': IssueType.ERROR_PRONE, # Forbid lambda inside loops. + 'WPS432': IssueType.INFO, # Forbid magic numbers. + 'WPS433': IssueType.CODE_STYLE, # Forbid imports nested in functions. + 'WPS439': IssueType.ERROR_PRONE, # Forbid Unicode escape sequences in binary strings. + 'WPS440': IssueType.ERROR_PRONE, # Forbid overlapping local and block variables. + 'WPS441': IssueType.ERROR_PRONE, # Forbid control variables after the block body. + 'WPS442': IssueType.ERROR_PRONE, # Forbid shadowing variables from outer scopes. + 'WPS443': IssueType.ERROR_PRONE, # Forbid explicit unhashable types of asset items and dict keys. + 'WPS445': IssueType.ERROR_PRONE, # Forbid incorrectly named keywords in starred dicts. + 'WPS448': IssueType.ERROR_PRONE, # Forbid incorrect order of except. + 'WPS449': IssueType.ERROR_PRONE, # Forbid float keys. + 'WPS456': IssueType.ERROR_PRONE, # Forbids using float("NaN") construct to generate NaN. + 'WPS457': IssueType.ERROR_PRONE, # Forbids use of infinite while True: loops. + 'WPS458': IssueType.ERROR_PRONE, # Forbids to import from already imported modules. + + # WPS: Refactoring + 'WPS524': IssueType.ERROR_PRONE, # Forbid misrefactored self assignment. + + # WPS: OOP + 'WPS601': IssueType.ERROR_PRONE, # Forbid shadowing class level attributes with instance level attributes. + 'WPS613': IssueType.ERROR_PRONE, # Forbid super() with incorrect method or property access. + 'WPS614': IssueType.ERROR_PRONE, # Forbids descriptors in regular functions. } CODE_PREFIX_TO_ISSUE_TYPE: Dict[str, IssueType] = { 'B': IssueType.ERROR_PRONE, # flake8-bugbear 'A': IssueType.ERROR_PRONE, # flake8-builtins 'R': IssueType.ERROR_PRONE, # flake8-return + 'P': IssueType.ERROR_PRONE, # flake8-format-string 'E': IssueType.CODE_STYLE, # standard flake8 'W': IssueType.CODE_STYLE, # standard flake8 @@ -30,4 +110,11 @@ 'F': IssueType.BEST_PRACTICES, # standard flake8 'C': IssueType.BEST_PRACTICES, # flake8-comprehensions 'SC': IssueType.INFO, # flake8-spellcheck + + 'WPS1': IssueType.CODE_STYLE, # WPS type: Naming + 'WPS2': IssueType.COMPLEXITY, # WPS type: Complexity + 'WPS3': IssueType.BEST_PRACTICES, # WPS type: Consistency + 'WPS4': IssueType.BEST_PRACTICES, # WPS type: Best practices + 'WPS5': IssueType.BEST_PRACTICES, # WPS type: Refactoring + 'WPS6': IssueType.BEST_PRACTICES, # WPS type: OOP } diff --git a/src/python/review/inspectors/flake8/whitelist.txt b/src/python/review/inspectors/flake8/whitelist.txt index 8ab08f11..38715032 100644 --- a/src/python/review/inspectors/flake8/whitelist.txt +++ b/src/python/review/inspectors/flake8/whitelist.txt @@ -1,9 +1,12 @@ aggfunc appendleft +arange argmax asctime astype +barmode betavariate +bgcolor birthdate blackbox bs4 @@ -13,6 +16,8 @@ capwords casefold caseless concat +config +configs consts coord copysign @@ -40,6 +45,7 @@ expm1 falsy fillna floordiv +fromkeys fromstring fullmatch gensim @@ -47,6 +53,7 @@ gmtime groupby halfs hashable +hline href hyp hyperskill @@ -72,12 +79,16 @@ lemmatizer lifes lim linalg +linecolor +linewidth linspace lowercased lvl lxml matmul +minsize multiline +nbins ndarray ndigits ndim @@ -99,6 +110,7 @@ rindex rmdir schur scipy +showline sigmoid sqrt src @@ -109,19 +121,27 @@ subdir subdirs substr substring +textposition textwrap todos tokenize tokenized tokenizer tolist +toplevel tracklist truediv truthy +uniformtext unpickled upd util utils +vline webpage whitespaces -writeback \ No newline at end of file +writeback +xanchor +xaxes +yanchor +yaxis diff --git a/src/python/review/inspectors/inspector_type.py b/src/python/review/inspectors/inspector_type.py index 1b69ac2d..15482a8b 100644 --- a/src/python/review/inspectors/inspector_type.py +++ b/src/python/review/inspectors/inspector_type.py @@ -8,6 +8,7 @@ class InspectorType(Enum): PYLINT = 'PYLINT' PYTHON_AST = 'PYTHON_AST' FLAKE8 = 'FLAKE8' + RADON = 'RADON' # Java language PMD = 'PMD' @@ -29,6 +30,7 @@ def available_values(cls) -> List[str]: cls.PYLINT.value, cls.FLAKE8.value, cls.PYTHON_AST.value, + cls.RADON.value, # Java language cls.PMD.value, diff --git a/src/python/review/inspectors/intellij/intellij.py b/src/python/review/inspectors/intellij/intellij.py index 8b962947..b50dc0ca 100644 --- a/src/python/review/inspectors/intellij/intellij.py +++ b/src/python/review/inspectors/intellij/intellij.py @@ -48,7 +48,7 @@ def __init__(self): def create_command(output_dir_path) -> List[Union[str, Path]]: return [ INTELLIJ_INSPECTOR_EXECUTABLE, INTELLIJ_INSPECTOR_PROJECT, - INTELLIJ_INSPECTOR_SETTINGS, output_dir_path, '-v2' + INTELLIJ_INSPECTOR_SETTINGS, output_dir_path, '-v2', ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -134,8 +134,8 @@ def parse(cls, out_dir_path: Path, file_path = Path( text.replace( 'file://$PROJECT_DIR$', - str(INTELLIJ_INSPECTOR_PROJECT) - ) + str(INTELLIJ_INSPECTOR_PROJECT), + ), ) elif tag == 'line': line_no = int(text) @@ -160,7 +160,7 @@ def parse(cls, out_dir_path: Path, description=description, origin_class=issue_class, inspector_type=cls.inspector_type, - type=issue_type + type=issue_type, )) return issues diff --git a/src/python/review/inspectors/intellij/issue_types/__init__.py b/src/python/review/inspectors/intellij/issue_types/__init__.py index c6f21127..97b8d31e 100644 --- a/src/python/review/inspectors/intellij/issue_types/__init__.py +++ b/src/python/review/inspectors/intellij/issue_types/__init__.py @@ -1,15 +1,18 @@ from typing import Dict -from src.python.review import IssueType -from .java import ISSUE_CLASS_TO_ISSUE_TYPE as \ - JAVA_ISSUE_CLASS_TO_ISSUE_TYPE -from .kotlin import ISSUE_CLASS_TO_ISSUE_TYPE as \ - KOTLIN_ISSUE_CLASS_TO_ISSUE_TYPE -from .python import ISSUE_CLASS_TO_ISSUE_TYPE as \ - PYTHON_ISSUE_CLASS_TO_ISSUE_TYPE +from src.python.review.inspectors.intellij.issue_types.java import ( + ISSUE_CLASS_TO_ISSUE_TYPE as JAVA_ISSUE_CLASS_TO_ISSUE_TYPE, +) +from src.python.review.inspectors.intellij.issue_types.kotlin import ( + ISSUE_CLASS_TO_ISSUE_TYPE as KOTLIN_ISSUE_CLASS_TO_ISSUE_TYPE, +) +from src.python.review.inspectors.intellij.issue_types.python import ( + ISSUE_CLASS_TO_ISSUE_TYPE as PYTHON_ISSUE_CLASS_TO_ISSUE_TYPE, +) +from src.python.review.inspectors.issue import IssueType ISSUE_CLASS_TO_ISSUE_TYPE: Dict[str, IssueType] = { **JAVA_ISSUE_CLASS_TO_ISSUE_TYPE, **PYTHON_ISSUE_CLASS_TO_ISSUE_TYPE, - **KOTLIN_ISSUE_CLASS_TO_ISSUE_TYPE + **KOTLIN_ISSUE_CLASS_TO_ISSUE_TYPE, } diff --git a/src/python/review/inspectors/intellij/issue_types/kotlin.py b/src/python/review/inspectors/intellij/issue_types/kotlin.py index 97438dca..cb692510 100644 --- a/src/python/review/inspectors/intellij/issue_types/kotlin.py +++ b/src/python/review/inspectors/intellij/issue_types/kotlin.py @@ -378,5 +378,5 @@ '\'when\' that can be simplified by introducing an argument': IssueType.CODE_STYLE, - 'Annotator': IssueType.ERROR_PRONE + 'Annotator': IssueType.ERROR_PRONE, } diff --git a/src/python/review/inspectors/issue.py b/src/python/review/inspectors/issue.py index 3174d7c6..b1b3cf52 100644 --- a/src/python/review/inspectors/issue.py +++ b/src/python/review/inspectors/issue.py @@ -25,6 +25,7 @@ class IssueType(Enum): COHESION = 'COHESION' CLASS_RESPONSE = 'CLASS_RESPONSE' METHOD_NUMBER = 'METHOD_NUMBER' + MAINTAINABILITY = 'MAINTAINABILITY' INFO = 'INFO' @@ -46,6 +47,8 @@ class IssueData(Enum): FUNCTION_LEN = 'func_len' BOOL_EXPR_LEN = 'bool_expr_len' CYCLOMATIC_COMPLEXITY = 'cc_value' + COHESION_LACK = 'cohesion_lack' + MAINTAINABILITY_LACK = 'maintainability_lack' @classmethod def get_base_issue_data_dict(cls, @@ -59,7 +62,7 @@ def get_base_issue_data_dict(cls, cls.LINE_NUMBER.value: line_number, cls.COLUMN_NUMBER.value: column_number, cls.ORIGIN_ClASS.value: origin_class, - cls.INSPECTOR_TYPE.value: inspector_type + cls.INSPECTOR_TYPE.value: inspector_type, } @@ -184,3 +187,12 @@ class MethodNumberIssue(BaseIssue, Measurable): def measure(self) -> int: return self.method_number + + +@dataclass(frozen=True) +class MaintainabilityLackIssue(BaseIssue, Measurable): + maintainability_lack: int + type = IssueType.MAINTAINABILITY + + def measure(self) -> int: + return self.maintainability_lack diff --git a/src/python/review/inspectors/parsers/checkstyle_parser.py b/src/python/review/inspectors/parsers/checkstyle_parser.py index 9cf75acb..1db9874c 100644 --- a/src/python/review/inspectors/parsers/checkstyle_parser.py +++ b/src/python/review/inspectors/parsers/checkstyle_parser.py @@ -1,15 +1,27 @@ import logging import re from pathlib import Path -from typing import Callable, Dict, List, Any, Optional +from typing import Any, Callable, Dict, List, Optional from xml.etree import ElementTree from src.python.review.common.file_system import get_content_from_file from src.python.review.inspectors.inspector_type import InspectorType -from src.python.review.inspectors.issue import BaseIssue, BoolExprLenIssue, CodeIssue, CyclomaticComplexityIssue, \ - FuncLenIssue, IssueType, LineLenIssue, IssueData -from src.python.review.inspectors.tips import get_bool_expr_len_tip, get_cyclomatic_complexity_tip, get_func_len_tip, \ - get_line_len_tip +from src.python.review.inspectors.issue import ( + BaseIssue, + BoolExprLenIssue, + CodeIssue, + CyclomaticComplexityIssue, + FuncLenIssue, + IssueData, + IssueType, + LineLenIssue, +) +from src.python.review.inspectors.tips import ( + get_bool_expr_len_tip, + get_cyclomatic_complexity_tip, + get_func_len_tip, + get_line_len_tip, +) logger = logging.getLogger(__name__) diff --git a/src/python/review/inspectors/pmd/files/bin/basic.xml b/src/python/review/inspectors/pmd/files/bin/basic.xml index 6cacd764..612bbf0e 100644 --- a/src/python/review/inspectors/pmd/files/bin/basic.xml +++ b/src/python/review/inspectors/pmd/files/bin/basic.xml @@ -46,29 +46,35 @@ - + - + + - + - + - - + + @@ -90,7 +96,8 @@ - + diff --git a/src/python/review/inspectors/pmd/issue_types.py b/src/python/review/inspectors/pmd/issue_types.py index e4609146..2188702b 100644 --- a/src/python/review/inspectors/pmd/issue_types.py +++ b/src/python/review/inspectors/pmd/issue_types.py @@ -2,7 +2,7 @@ from src.python.review.inspectors.issue import IssueType -RULE_TO_ISSUE_TYPE: Dict[str, IssueType] = { +PMD_RULE_TO_ISSUE_TYPE: Dict[str, IssueType] = { # Best Practices 'AbstractClassWithoutAbstractMethod': IssueType.BEST_PRACTICES, 'AccessorClassGeneration': IssueType.BEST_PRACTICES, diff --git a/src/python/review/inspectors/pmd/pmd.py b/src/python/review/inspectors/pmd/pmd.py index 227169fc..eb4878dd 100644 --- a/src/python/review/inspectors/pmd/pmd.py +++ b/src/python/review/inspectors/pmd/pmd.py @@ -8,15 +8,17 @@ from src.python.review.common.file_system import new_temp_dir from src.python.review.common.subprocess_runner import run_in_subprocess from src.python.review.inspectors.base_inspector import BaseInspector +from src.python.review.inspectors.common import remove_prefix from src.python.review.inspectors.inspector_type import InspectorType from src.python.review.inspectors.issue import BaseIssue, CodeIssue, IssueType -from src.python.review.inspectors.pmd.issue_types import RULE_TO_ISSUE_TYPE +from src.python.review.inspectors.pmd.issue_types import PMD_RULE_TO_ISSUE_TYPE logger = logging.getLogger(__name__) PATH_TOOLS_PMD_FILES = Path(__file__).parent / 'files' PATH_TOOLS_PMD_SHELL_SCRIPT = PATH_TOOLS_PMD_FILES / 'bin' / 'run.sh' PATH_TOOLS_PMD_RULES_SET = PATH_TOOLS_PMD_FILES / 'bin' / 'basic.xml' +DEFAULT_JAVA_VERSION = LanguageVersion.JAVA_11 class PMDInspector(BaseInspector): @@ -28,16 +30,16 @@ def __init__(self): @classmethod def _create_command(cls, path: Path, output_path: Path, - java_version: LanguageVersion, + language_version: LanguageVersion, n_cpu: int) -> List[str]: return [ PATH_TOOLS_PMD_SHELL_SCRIPT, 'pmd', '-d', str(path), '-no-cache', '-R', PATH_TOOLS_PMD_RULES_SET, '-language', 'java', - '-version', java_version.value, + '-version', cls._get_java_version(language_version), '-f', 'csv', '-r', str(output_path), - '-t', str(n_cpu) + '-t', str(n_cpu), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -46,13 +48,21 @@ def inspect(self, path: Path, config: dict) -> List[BaseIssue]: language_version = config.get('language_version') if language_version is None: - language_version = LanguageVersion.JAVA_11 + logger.info( + f"The version of Java is not passed. The version to be used is: {DEFAULT_JAVA_VERSION.value}.", + ) + language_version = DEFAULT_JAVA_VERSION command = self._create_command(path, output_path, language_version, config['n_cpu']) run_in_subprocess(command) return self.parse_output(output_path) def parse_output(self, output_path: Path) -> List[BaseIssue]: + """ + Parses the PMD output, which is a csv file, and returns a list of the issues found there. + + If the passed path is not a file, an empty list is returned. + """ if not output_path.is_file(): logger.error('%s: error - no output file' % self.inspector_type.value) return [] @@ -65,17 +75,37 @@ def parse_output(self, output_path: Path) -> List[BaseIssue]: line_no=int(row['Line']), column_no=1, type=self.choose_issue_type(row['Rule']), - origin_class=row['Rule set'], + origin_class=row['Rule'], description=row['Description'], inspector_type=self.inspector_type, ) for row in reader] @classmethod def choose_issue_type(cls, rule: str) -> IssueType: - issue_type = RULE_TO_ISSUE_TYPE.get(rule) + """ + Defines IssueType by PMD rule name using config. + """ + issue_type = PMD_RULE_TO_ISSUE_TYPE.get(rule) if not issue_type: logger.warning('%s: %s - unknown rule' % (cls.inspector_type.value, rule)) return IssueType.BEST_PRACTICES return issue_type + + @staticmethod + def _get_java_version(language_version: LanguageVersion) -> str: + """ + Converts language_version to the version of Java that PMD can work with. + + For example, java11 will be converted to 11. + """ + java_version = language_version.value + + if not language_version.is_java(): + logger.warning( + f"The version passed is not the Java version. The version to be used is: {DEFAULT_JAVA_VERSION.value}.", + ) + java_version = DEFAULT_JAVA_VERSION.value + + return remove_prefix(java_version, "java") diff --git a/src/python/review/inspectors/pyast/python_ast.py b/src/python/review/inspectors/pyast/python_ast.py index 28b76e9f..6426d115 100644 --- a/src/python/review/inspectors/pyast/python_ast.py +++ b/src/python/review/inspectors/pyast/python_ast.py @@ -40,7 +40,7 @@ def visit(self, node: ast.AST): origin_class=BOOL_EXPR_LEN_ORIGIN_CLASS, inspector_type=self._inspector_type, bool_expr_len=length, - type=IssueType.BOOL_EXPR_LEN + type=IssueType.BOOL_EXPR_LEN, )) @@ -58,7 +58,7 @@ def __init__(self, content: str, file_path: Path, inspector_type: InspectorType) def visit(self, node): if isinstance(self._previous_node, (ast.FunctionDef, ast.AsyncFunctionDef)): func_length = self._find_func_len( - self._previous_node.lineno, node.lineno + self._previous_node.lineno, node.lineno, ) self._function_lens.append(FuncLenIssue( @@ -69,7 +69,7 @@ def visit(self, node): origin_class=FUNC_LEN_ORIGIN_CLASS, inspector_type=self._inspector_type, func_len=func_length, - type=IssueType.FUNC_LEN + type=IssueType.FUNC_LEN, )) self._previous_node = node @@ -80,7 +80,7 @@ def visit(self, node): def function_lens(self) -> List[FuncLenIssue]: if isinstance(self._previous_node, (ast.FunctionDef, ast.AsyncFunctionDef)): func_length = self._find_func_len( - self._previous_node.lineno, self._n_lines + 1 + self._previous_node.lineno, self._n_lines + 1, ) self._function_lens.append(FuncLenIssue( @@ -91,7 +91,7 @@ def function_lens(self) -> List[FuncLenIssue]: origin_class=FUNC_LEN_ORIGIN_CLASS, inspector_type=self._inspector_type, func_len=func_length, - type=IssueType.FUNC_LEN + type=IssueType.FUNC_LEN, )) self._previous_node = None @@ -125,13 +125,13 @@ def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: bool_gatherer = BoolExpressionLensGatherer(path_to_file, cls.inspector_type) bool_gatherer.visit(tree) metrics.extend( - bool_gatherer.bool_expression_lens + bool_gatherer.bool_expression_lens, ) func_gatherer = FunctionLensGatherer(file_content, path_to_file, cls.inspector_type) func_gatherer.visit(tree) metrics.extend( - func_gatherer.function_lens + func_gatherer.function_lens, ) return metrics diff --git a/src/python/review/inspectors/pylint/issue_types.py b/src/python/review/inspectors/pylint/issue_types.py index edfe8818..5df676b1 100644 --- a/src/python/review/inspectors/pylint/issue_types.py +++ b/src/python/review/inspectors/pylint/issue_types.py @@ -8,6 +8,8 @@ # E errors, for probable bugs in the code CODE_TO_ISSUE_TYPE: Dict[str, IssueType] = { + 'C0200': IssueType.BEST_PRACTICES, # consider using enumerate + 'C0415': IssueType.BEST_PRACTICES, # import-outside-toplevel 'W0101': IssueType.ERROR_PRONE, # unreachable code 'W0102': IssueType.ERROR_PRONE, # dangerous default value 'W0104': IssueType.ERROR_PRONE, # statement doesn't have any effect diff --git a/src/python/review/inspectors/pylint/pylint.py b/src/python/review/inspectors/pylint/pylint.py index fb9db581..e5a256cb 100644 --- a/src/python/review/inspectors/pylint/pylint.py +++ b/src/python/review/inspectors/pylint/pylint.py @@ -21,7 +21,7 @@ class PylintInspector(BaseInspector): supported_issue_types = ( IssueType.CODE_STYLE, IssueType.BEST_PRACTICES, - IssueType.ERROR_PRONE + IssueType.ERROR_PRONE, ) @classmethod @@ -31,7 +31,7 @@ def inspect(cls, path: Path, config: dict) -> List[CodeIssue]: '--load-plugins', 'pylint_django', f'--rcfile={PATH_PYLINT_CONFIG}', f'--msg-template={MSG_TEMPLATE}', - str(path) + str(path), ] output = run_in_subprocess(command) @@ -70,7 +70,7 @@ def parse(cls, output: str) -> List[CodeIssue]: origin_class=origin_class, description=description, inspector_type=cls.inspector_type, - type=issue_type + type=issue_type, )) return issues diff --git a/src/python/review/inspectors/pylint/pylintrc b/src/python/review/inspectors/pylint/pylintrc index c435b63b..357ba058 100644 --- a/src/python/review/inspectors/pylint/pylintrc +++ b/src/python/review/inspectors/pylint/pylintrc @@ -164,6 +164,8 @@ disable=invalid-name, no-else-raise, no-else-break, no-else-continue, + W0614, + W0622, # Enable the message, report, category or checker with the given id(s). You can # either give multiple identifier separated by comma (,) or put this option diff --git a/test/resources/inspectors/python/__init__.py b/src/python/review/inspectors/radon/__init__.py similarity index 100% rename from test/resources/inspectors/python/__init__.py rename to src/python/review/inspectors/radon/__init__.py diff --git a/src/python/review/inspectors/radon/radon.py b/src/python/review/inspectors/radon/radon.py new file mode 100644 index 00000000..79223164 --- /dev/null +++ b/src/python/review/inspectors/radon/radon.py @@ -0,0 +1,56 @@ +import re +from pathlib import Path +from typing import List + +from src.python.review.common.subprocess_runner import run_in_subprocess +from src.python.review.inspectors.base_inspector import BaseInspector +from src.python.review.inspectors.common import convert_percentage_of_value_to_lack_of_value +from src.python.review.inspectors.inspector_type import InspectorType +from src.python.review.inspectors.issue import BaseIssue, IssueData, IssueType, MaintainabilityLackIssue +from src.python.review.inspectors.tips import get_maintainability_index_tip + + +MAINTAINABILITY_ORIGIN_CLASS = "RAD100" + + +class RadonInspector(BaseInspector): + inspector_type = InspectorType.RADON + + @classmethod + def inspect(cls, path: Path, config: dict) -> List[BaseIssue]: + mi_command = [ + "radon", "mi", # compute the Maintainability Index score + "--max", "F", # set the maximum MI rank to display + "--show", # actual MI value is shown in results, alongside the rank + path, + ] + + mi_output = run_in_subprocess(mi_command) + return cls.mi_parse(mi_output) + + @classmethod + def mi_parse(cls, mi_output: str) -> List[BaseIssue]: + """ + Parses the results of the "mi" command. + Description: https://radon.readthedocs.io/en/latest/commandline.html#the-mi-command + + :param mi_output: "mi" command output. + :return: list of issues. + """ + row_re = re.compile(r"^(.*) - \w \((.*)\)$", re.M) + + issues: List[BaseIssue] = [] + for groups in row_re.findall(mi_output): + file_path = Path(groups[0]) + maintainability_lack = convert_percentage_of_value_to_lack_of_value(float(groups[1])) + + issue_data = IssueData.get_base_issue_data_dict( + file_path, cls.inspector_type, origin_class=MAINTAINABILITY_ORIGIN_CLASS, + ) + issue_data[IssueData.DESCRIPTION.value] = get_maintainability_index_tip() + issue_data[IssueData.MAINTAINABILITY_LACK.value] = maintainability_lack + issue_data[IssueData.ISSUE_TYPE.value] = IssueType.MAINTAINABILITY + + issues.append(MaintainabilityLackIssue(**issue_data)) + + return issues diff --git a/src/python/review/inspectors/spotbugs/spotbugs.py b/src/python/review/inspectors/spotbugs/spotbugs.py index e9cd1cb2..a2ee8b38 100644 --- a/src/python/review/inspectors/spotbugs/spotbugs.py +++ b/src/python/review/inspectors/spotbugs/spotbugs.py @@ -30,7 +30,7 @@ def _create_command(cls, path: Path) -> List[str]: PATH_SPOTBUGS_EXCLUDE, '-textui', '-medium', - str(path) + str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -100,5 +100,5 @@ def _parse_single_line(cls, line: str, file_name_to_path: Dict[str, Path]) -> Ba type=IssueType.ERROR_PRONE, origin_class=issue_class, description=short_desc, - inspector_type=cls.inspector_type + inspector_type=cls.inspector_type, ) diff --git a/src/python/review/inspectors/springlint/springlint.py b/src/python/review/inspectors/springlint/springlint.py index 18f9d6b0..b5eca5a4 100644 --- a/src/python/review/inspectors/springlint/springlint.py +++ b/src/python/review/inspectors/springlint/springlint.py @@ -3,17 +3,34 @@ import re from pathlib import Path from shutil import copy -from typing import AnyStr, List, Optional, Dict, Any +from typing import Any, AnyStr, Dict, List, Optional from src.python.review.common.file_system import new_temp_dir from src.python.review.common.subprocess_runner import run_in_subprocess from src.python.review.inspectors.base_inspector import BaseInspector from src.python.review.inspectors.inspector_type import InspectorType -from src.python.review.inspectors.issue import BaseIssue, ChildrenNumberIssue, ClassResponseIssue, CodeIssue, \ - CohesionIssue, \ - CouplingIssue, InheritanceIssue, IssueType, MethodNumberIssue, WeightedMethodIssue, IssueData -from src.python.review.inspectors.tips import get_child_number_tip, get_class_coupling_tip, get_class_response_tip, \ - get_cohesion_tip, get_inheritance_depth_tip, get_method_number_tip, get_weighted_method_tip +from src.python.review.inspectors.issue import ( + BaseIssue, + ChildrenNumberIssue, + ClassResponseIssue, + CodeIssue, + CohesionIssue, + CouplingIssue, + InheritanceIssue, + IssueData, + IssueType, + MethodNumberIssue, + WeightedMethodIssue, +) +from src.python.review.inspectors.tips import ( + get_child_number_tip, + get_class_coupling_tip, + get_class_response_tip, + get_cohesion_tip, + get_inheritance_depth_tip, + get_method_number_tip, + get_weighted_method_tip, +) PATH_TOOLS_SPRINGLINT_FILES = Path(__file__).parent / 'files' PATH_SPRINGLINT_JAR = PATH_TOOLS_SPRINGLINT_FILES / 'springlint-0.6.jar' @@ -32,7 +49,7 @@ class SpringlintInspector(BaseInspector): 'cbo': 'class_objects_coupling', 'lcom': 'cohesion_lack', 'rfc': 'class_response', - 'nom': 'method_number' + 'nom': 'method_number', } metric_name_to_description = { @@ -42,7 +59,7 @@ class SpringlintInspector(BaseInspector): 'cbo': get_class_coupling_tip(), 'lcom': get_cohesion_tip(), 'rfc': get_class_response_tip(), - 'nom': get_method_number_tip() + 'nom': get_method_number_tip(), } metric_name_to_issue_type = { @@ -52,7 +69,7 @@ class SpringlintInspector(BaseInspector): 'cbo': IssueType.COUPLING, 'lcom': IssueType.COHESION, 'rfc': IssueType.CLASS_RESPONSE, - 'nom': IssueType.METHOD_NUMBER + 'nom': IssueType.METHOD_NUMBER, } @classmethod @@ -62,7 +79,7 @@ def _create_command(cls, path: Path, output_path: Path) -> List[str]: PATH_SPRINGLINT_JAR, '--output', str(output_path), '-otype', 'html', - '--project', str(path) + '--project', str(path), ] def inspect(self, path: Path, config: dict) -> List[BaseIssue]: @@ -118,7 +135,7 @@ def _parse_smells(cls, file_content: AnyStr, origin_path: str = '') -> List[Base origin_class=smell['name'], inspector_type=cls.inspector_type, type=IssueType.ARCHITECTURE, - description=smell['description'] + description=smell['description'], ) for smell in file_smell['smells']]) return issues diff --git a/src/python/review/inspectors/tips.py b/src/python/review/inspectors/tips.py index c1d3fba1..eac449c1 100644 --- a/src/python/review/inspectors/tips.py +++ b/src/python/review/inspectors/tips.py @@ -1,24 +1,32 @@ def get_bool_expr_len_tip() -> str: - return 'Too long boolean expression. ' \ - 'Try to split it into smaller expressions.' + return ( + 'Too long boolean expression. ' + 'Try to split it into smaller expressions.' + ) def get_func_len_tip() -> str: - return 'Too long function. ' \ - 'Try to split it into smaller functions / methods.' \ - 'It will make your code easy to understand and less error prone.' + return ( + 'Too long function. ' + 'Try to split it into smaller functions / methods. ' + 'It will make your code easy to understand and less error prone.' + ) def get_line_len_tip() -> str: - return 'Too long line. ' \ - 'Try to split it into smaller lines.' \ - 'It will make your code easy to understand.' + return ( + 'Too long line. ' + 'Try to split it into smaller lines. ' + 'It will make your code easy to understand.' + ) def get_cyclomatic_complexity_tip() -> str: - return 'Too complex function. You can figure out how to simplify this code ' \ - 'or split it into a set of small functions / methods. ' \ - 'It will make your code easy to understand and less error prone.' + return ( + 'Too complex function. You can figure out how to simplify this code ' + 'or split it into a set of small functions / methods. ' + 'It will make your code easy to understand and less error prone.' + ) def add_complexity_tip(description: str) -> str: @@ -31,8 +39,10 @@ def add_complexity_tip(description: str) -> str: def get_inheritance_depth_tip() -> str: - return 'Too deep inheritance tree is complicated to understand. ' \ - 'Try to reduce it (maybe you should use a composition instead).' + return ( + 'Too deep inheritance tree is complicated to understand. ' + 'Try to reduce it (maybe you should use a composition instead).' + ) # This issue will not be reported at this version @@ -41,14 +51,18 @@ def get_child_number_tip() -> str: def get_weighted_method_tip() -> str: - return 'The number of methods and their complexity may be too hight. ' \ - 'It may require too much time and effort to develop and maintain the class.' + return ( + 'The number of methods and their complexity may be too hight. ' + 'It may require too much time and effort to develop and maintain the class.' + ) def get_class_coupling_tip() -> str: - return 'The class seems to depend on too many other classes. ' \ - 'Increased coupling increases interclass dependencies, ' \ - 'making the code less modular and less suitable for reuse and testing.' + return ( + 'The class seems to depend on too many other classes. ' + 'Increased coupling increases interclass dependencies, ' + 'making the code less modular and less suitable for reuse and testing.' + ) # This issue will not be reported at this version @@ -57,12 +71,21 @@ def get_cohesion_tip() -> str: def get_class_response_tip() -> str: - return 'The class have too many methods that can potentially ' \ - 'be executed in response to a single message received by an object of that class. ' \ - 'The larger the number of methods that can be invoked from a class, ' \ - 'the greater the complexity of the class' + return ( + 'The class have too many methods that can potentially ' + 'be executed in response to a single message received by an object of that class. ' + 'The larger the number of methods that can be invoked from a class, ' + 'the greater the complexity of the class' + ) def get_method_number_tip() -> str: - return 'The file has too many methods inside and is complicated to understand. ' \ - 'Consider its decomposition to smaller classes.' + return ( + 'The file has too many methods inside and is complicated to understand. ' + 'Consider its decomposition to smaller classes.' + ) + + +# TODO: Need to improve the tip. +def get_maintainability_index_tip() -> str: + return 'The maintainability index is too low.' diff --git a/src/python/review/logging_config.py b/src/python/review/logging_config.py index ce3c47a0..ee55bf46 100644 --- a/src/python/review/logging_config.py +++ b/src/python/review/logging_config.py @@ -5,22 +5,22 @@ 'formatters': { 'common': { 'class': 'logging.Formatter', - 'format': '%(asctime)s | %(levelname)s | %(message)s' - } + 'format': '%(asctime)s | %(levelname)s | %(message)s', + }, }, 'handlers': { 'console': { 'class': 'logging.StreamHandler', 'level': 'DEBUG', 'formatter': 'common', - 'stream': sys.stdout + 'stream': sys.stdout, }, }, 'loggers': { '': { 'handlers': ['console'], - 'level': 'INFO' - } + 'level': 'INFO', + }, }, - 'disable_existing_loggers': False + 'disable_existing_loggers': False, } diff --git a/src/python/review/quality/evaluate_quality.py b/src/python/review/quality/evaluate_quality.py index 3d78e392..b6329653 100644 --- a/src/python/review/quality/evaluate_quality.py +++ b/src/python/review/quality/evaluate_quality.py @@ -4,25 +4,46 @@ from src.python.review.inspectors.issue import IssueType from src.python.review.quality.model import Quality, Rule from src.python.review.quality.rules.best_practices_scoring import ( - BestPracticesRule, LANGUAGE_TO_BEST_PRACTICES_RULE_CONFIG + BestPracticesRule, + LANGUAGE_TO_BEST_PRACTICES_RULE_CONFIG, +) +from src.python.review.quality.rules.boolean_length_scoring import ( + BooleanExpressionRule, + LANGUAGE_TO_BOOLEAN_EXPRESSION_RULE_CONFIG, ) -from src.python.review.quality.rules.boolean_length_scoring import BooleanExpressionRule, \ - LANGUAGE_TO_BOOLEAN_EXPRESSION_RULE_CONFIG from src.python.review.quality.rules.class_response_scoring import LANGUAGE_TO_RESPONSE_RULE_CONFIG, ResponseRule from src.python.review.quality.rules.code_style_scoring import CodeStyleRule, LANGUAGE_TO_CODE_STYLE_RULE_CONFIG +from src.python.review.quality.rules.cohesion_scoring import ( + CohesionRule, + LANGUAGE_TO_COHESION_RULE_CONFIG, +) from src.python.review.quality.rules.coupling_scoring import CouplingRule, LANGUAGE_TO_COUPLING_RULE_CONFIG -from src.python.review.quality.rules.cyclomatic_complexity_scoring import CyclomaticComplexityRule, \ - LANGUAGE_TO_CYCLOMATIC_COMPLEXITY_RULE_CONFIG +from src.python.review.quality.rules.cyclomatic_complexity_scoring import ( + CyclomaticComplexityRule, + LANGUAGE_TO_CYCLOMATIC_COMPLEXITY_RULE_CONFIG, +) from src.python.review.quality.rules.error_prone_scoring import ErrorProneRule, LANGUAGE_TO_ERROR_PRONE_RULE_CONFIG -from src.python.review.quality.rules.function_length_scoring import FunctionLengthRule, \ - LANGUAGE_TO_FUNCTION_LENGTH_RULE_CONFIG -from src.python.review.quality.rules.inheritance_depth_scoring import InheritanceDepthRule, \ - LANGUAGE_TO_INHERITANCE_DEPTH_RULE_CONFIG +from src.python.review.quality.rules.function_length_scoring import ( + FunctionLengthRule, + LANGUAGE_TO_FUNCTION_LENGTH_RULE_CONFIG, +) +from src.python.review.quality.rules.inheritance_depth_scoring import ( + InheritanceDepthRule, + LANGUAGE_TO_INHERITANCE_DEPTH_RULE_CONFIG, +) from src.python.review.quality.rules.line_len_scoring import LANGUAGE_TO_LINE_LENGTH_RULE_CONFIG, LineLengthRule -from src.python.review.quality.rules.method_number_scoring import LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG, \ - MethodNumberRule -from src.python.review.quality.rules.weighted_methods_scoring import LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG, \ - WeightedMethodsRule +from src.python.review.quality.rules.maintainability_scoring import ( + LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG, + MaintainabilityRule, +) +from src.python.review.quality.rules.method_number_scoring import ( + LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG, + MethodNumberRule, +) +from src.python.review.quality.rules.weighted_methods_scoring import ( + LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG, + WeightedMethodsRule, +) from src.python.review.reviewers.utils.code_statistics import CodeStatistics @@ -37,7 +58,9 @@ def __get_available_rules(language: Language) -> List[Rule]: MethodNumberRule(LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG[language]), CouplingRule(LANGUAGE_TO_COUPLING_RULE_CONFIG[language]), ResponseRule(LANGUAGE_TO_RESPONSE_RULE_CONFIG[language]), - WeightedMethodsRule(LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG[language]) + WeightedMethodsRule(LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG[language]), + CohesionRule(LANGUAGE_TO_COHESION_RULE_CONFIG[language]), + MaintainabilityRule(LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG[language]), ] diff --git a/src/python/review/quality/model.py b/src/python/review/quality/model.py index ae71f211..7a57dcf6 100644 --- a/src/python/review/quality/model.py +++ b/src/python/review/quality/model.py @@ -1,4 +1,5 @@ import abc +import textwrap from enum import Enum, unique from functools import total_ordering from typing import List @@ -19,7 +20,7 @@ def __le__(self, other: 'QualityType') -> bool: QualityType.BAD: 0, QualityType.MODERATE: 1, QualityType.GOOD: 2, - QualityType.EXCELLENT: 3 + QualityType.EXCELLENT: 3, } return order[self] < order[other] @@ -49,7 +50,7 @@ def quality_type(self) -> QualityType: def next_quality_type(self) -> QualityType: return min(map(lambda rule: rule.next_level_type, self.rules), default=QualityType.EXCELLENT) - # TODO@nbirillo: why rule.quality_type == quality_type for next level???? + # TODO: why rule.quality_type == quality_type for next level???? @property def next_level_requirements(self) -> List[Rule]: quality_type = self.quality_type @@ -80,8 +81,11 @@ def __str__(self): message_deltas_part = '' if self.quality_type != QualityType.EXCELLENT: - message_next_level_part = f'Next level: {self.next_quality_type.value}\n' \ - f'Next level requirements:\n' + message_next_level_part = f"""\ + Next level: {self.next_quality_type.value} + Next level requirements: + """ + message_next_level_part = textwrap.dedent(message_next_level_part) for rule in self.next_level_requirements: message_deltas_part += f'{rule.rule_type.value}: {rule.next_level_delta}\n' diff --git a/src/python/review/quality/rules/best_practices_scoring.py b/src/python/review/quality/rules/best_practices_scoring.py index f014d6e8..633387f2 100644 --- a/src/python/review/quality/rules/best_practices_scoring.py +++ b/src/python/review/quality/rules/best_practices_scoring.py @@ -16,7 +16,7 @@ class BestPracticesRuleConfig: common_best_practices_rule_config = BestPracticesRuleConfig( n_best_practices_moderate=3, n_best_practices_good=1, - n_files=1 + n_files=1, ) LANGUAGE_TO_BEST_PRACTICES_RULE_CONFIG = { @@ -59,7 +59,7 @@ def merge(self, other: 'BestPracticesRule') -> 'BestPracticesRule': config = BestPracticesRuleConfig( min(self.config.n_best_practices_moderate, other.config.n_best_practices_moderate), min(self.config.n_best_practices_good, other.config.n_best_practices_good), - n_files=self.config.n_files + other.config.n_files + n_files=self.config.n_files + other.config.n_files, ) result_rule = BestPracticesRule(config) result_rule.apply(self.n_best_practices + other.n_best_practices) diff --git a/src/python/review/quality/rules/boolean_length_scoring.py b/src/python/review/quality/rules/boolean_length_scoring.py index b2d7c347..2a33c327 100644 --- a/src/python/review/quality/rules/boolean_length_scoring.py +++ b/src/python/review/quality/rules/boolean_length_scoring.py @@ -16,13 +16,13 @@ class BooleanExpressionRuleConfig: common_boolean_expression_rule_config = BooleanExpressionRuleConfig( bool_expr_len_bad=10, bool_expr_len_moderate=7, - bool_expr_len_good=5 + bool_expr_len_good=5, ) java_boolean_expression_rule_config = BooleanExpressionRuleConfig( bool_expr_len_bad=8, bool_expr_len_moderate=6, - bool_expr_len_good=4 + bool_expr_len_good=4, ) LANGUAGE_TO_BOOLEAN_EXPRESSION_RULE_CONFIG = { @@ -66,7 +66,7 @@ def merge(self, other: 'BooleanExpressionRule') -> 'BooleanExpressionRule': config = BooleanExpressionRuleConfig( min(self.config.bool_expr_len_bad, other.config.bool_expr_len_bad), min(self.config.bool_expr_len_moderate, other.config.bool_expr_len_moderate), - min(self.config.bool_expr_len_good, other.config.bool_expr_len_good) + min(self.config.bool_expr_len_good, other.config.bool_expr_len_good), ) result_rule = BooleanExpressionRule(config) result_rule.apply(max(self.bool_expr_len, other.bool_expr_len)) diff --git a/src/python/review/quality/rules/class_response_scoring.py b/src/python/review/quality/rules/class_response_scoring.py index 4bfd4800..d4b82fa9 100644 --- a/src/python/review/quality/rules/class_response_scoring.py +++ b/src/python/review/quality/rules/class_response_scoring.py @@ -14,7 +14,7 @@ class ResponseRuleConfig: common_response_rule_config = ResponseRuleConfig( response_moderate=69, - response_good=59 + response_good=59, ) LANGUAGE_TO_RESPONSE_RULE_CONFIG = { @@ -52,7 +52,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'ResponseRule') -> 'ResponseRule': config = ResponseRuleConfig( min(self.config.response_moderate, other.config.response_moderate), - min(self.config.response_good, other.config.response_good) + min(self.config.response_good, other.config.response_good), ) result_rule = ResponseRule(config) result_rule.apply(max(self.response, other.response)) diff --git a/src/python/review/quality/rules/code_style_scoring.py b/src/python/review/quality/rules/code_style_scoring.py index 882b0c5c..a1edda09 100644 --- a/src/python/review/quality/rules/code_style_scoring.py +++ b/src/python/review/quality/rules/code_style_scoring.py @@ -19,7 +19,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.17, n_code_style_good=0, n_code_style_lines_bad=10, - language=Language.JAVA + language=Language.JAVA, ) python_code_style_rule_config = CodeStyleRuleConfig( @@ -27,7 +27,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.17, n_code_style_good=0, n_code_style_lines_bad=5, - language=Language.PYTHON + language=Language.PYTHON, ) kotlin_code_style_rule_config = CodeStyleRuleConfig( @@ -35,7 +35,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.07, n_code_style_good=0, n_code_style_lines_bad=10, - language=Language.KOTLIN + language=Language.KOTLIN, ) js_code_style_rule_config = CodeStyleRuleConfig( @@ -43,7 +43,7 @@ class CodeStyleRuleConfig: n_code_style_moderate=0.17, n_code_style_good=0, n_code_style_lines_bad=10, - language=Language.JAVA + language=Language.JAVA, ) LANGUAGE_TO_CODE_STYLE_RULE_CONFIG = { diff --git a/src/python/review/quality/rules/cohesion_scoring.py b/src/python/review/quality/rules/cohesion_scoring.py new file mode 100644 index 00000000..623fd2db --- /dev/null +++ b/src/python/review/quality/rules/cohesion_scoring.py @@ -0,0 +1,71 @@ +from dataclasses import dataclass +from typing import Optional + +from src.python.review.common.language import Language +from src.python.review.inspectors.issue import IssueType +from src.python.review.quality.model import QualityType, Rule + + +@dataclass +class CohesionRuleConfig: + cohesion_lack_bad: int + cohesion_lack_moderate: int + cohesion_lack_good: int + + +# TODO: Need testing +# Cohesion plugin by default only shows issues where cohesion is less than 50%. +# Therefore cohesion_lack_bad = 50. The other levels are set in steps of 20%. +common_cohesion_rule_config = CohesionRuleConfig( + cohesion_lack_bad=50, + cohesion_lack_moderate=30, + cohesion_lack_good=10, +) + +LANGUAGE_TO_COHESION_RULE_CONFIG = { + Language.JAVA: common_cohesion_rule_config, + Language.PYTHON: common_cohesion_rule_config, + Language.KOTLIN: common_cohesion_rule_config, + Language.JS: common_cohesion_rule_config, +} + + +class CohesionRule(Rule): + def __init__(self, config: CohesionRuleConfig): + self.config = config + self.rule_type = IssueType.COHESION + self.cohesion_lack: Optional[int] = None + + def apply(self, cohesion_lack: int): + self.cohesion_lack = cohesion_lack + if cohesion_lack > self.config.cohesion_lack_bad: + self.quality_type = QualityType.BAD + self.next_level_delta = cohesion_lack - self.config.cohesion_lack_bad + elif cohesion_lack > self.config.cohesion_lack_moderate: + self.quality_type = QualityType.MODERATE + self.next_level_delta = cohesion_lack - self.config.cohesion_lack_moderate + elif cohesion_lack > self.config.cohesion_lack_good: + self.quality_type = QualityType.GOOD + self.next_level_delta = cohesion_lack - self.config.cohesion_lack_good + else: + self.quality_type = QualityType.EXCELLENT + self.next_level_delta = 0 + self.next_level_type = self.__get_next_quality_type() + + def __get_next_quality_type(self) -> QualityType: + if self.quality_type == QualityType.BAD: + return QualityType.MODERATE + elif self.quality_type == QualityType.MODERATE: + return QualityType.GOOD + return QualityType.EXCELLENT + + def merge(self, other: 'CohesionRule') -> 'CohesionRule': + config = CohesionRuleConfig( + min(self.config.cohesion_lack_bad, other.config.cohesion_lack_bad), + min(self.config.cohesion_lack_moderate, other.config.cohesion_lack_moderate), + min(self.config.cohesion_lack_good, other.config.cohesion_lack_good), + ) + result_rule = CohesionRule(config) + result_rule.apply(max(self.cohesion_lack, other.cohesion_lack)) + + return result_rule diff --git a/src/python/review/quality/rules/coupling_scoring.py b/src/python/review/quality/rules/coupling_scoring.py index 83ea987b..7fb0a782 100644 --- a/src/python/review/quality/rules/coupling_scoring.py +++ b/src/python/review/quality/rules/coupling_scoring.py @@ -14,7 +14,7 @@ class CouplingRuleConfig: common_coupling_rule_config = CouplingRuleConfig( coupling_bad=30, - coupling_moderate=20 + coupling_moderate=20, ) LANGUAGE_TO_COUPLING_RULE_CONFIG = { @@ -52,7 +52,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'CouplingRule') -> 'CouplingRule': config = CouplingRuleConfig( min(self.config.coupling_bad, other.config.coupling_bad), - min(self.config.coupling_moderate, other.config.coupling_moderate) + min(self.config.coupling_moderate, other.config.coupling_moderate), ) result_rule = CouplingRule(config) result_rule.apply(max(self.coupling, other.coupling)) diff --git a/src/python/review/quality/rules/cyclomatic_complexity_scoring.py b/src/python/review/quality/rules/cyclomatic_complexity_scoring.py index 79fb69dc..162284b4 100644 --- a/src/python/review/quality/rules/cyclomatic_complexity_scoring.py +++ b/src/python/review/quality/rules/cyclomatic_complexity_scoring.py @@ -15,20 +15,20 @@ class CyclomaticComplexityRuleConfig: LANGUAGE_TO_CYCLOMATIC_COMPLEXITY_RULE_CONFIG = { Language.JAVA: CyclomaticComplexityRuleConfig( cc_value_bad=14, - cc_value_moderate=13 + cc_value_moderate=13, ), Language.KOTLIN: CyclomaticComplexityRuleConfig( cc_value_bad=12, - cc_value_moderate=11 + cc_value_moderate=11, ), Language.PYTHON: CyclomaticComplexityRuleConfig( cc_value_bad=10, - cc_value_moderate=9 + cc_value_moderate=9, ), Language.JS: CyclomaticComplexityRuleConfig( cc_value_bad=14, - cc_value_moderate=13 - ) + cc_value_moderate=13, + ), } diff --git a/src/python/review/quality/rules/error_prone_scoring.py b/src/python/review/quality/rules/error_prone_scoring.py index 7a6dee0e..8df9c157 100644 --- a/src/python/review/quality/rules/error_prone_scoring.py +++ b/src/python/review/quality/rules/error_prone_scoring.py @@ -12,7 +12,7 @@ class ErrorProneRuleConfig: common_error_prone_rule_config = ErrorProneRuleConfig( - n_error_prone_bad=0 + n_error_prone_bad=0, ) LANGUAGE_TO_ERROR_PRONE_RULE_CONFIG = { diff --git a/src/python/review/quality/rules/function_length_scoring.py b/src/python/review/quality/rules/function_length_scoring.py index f607f679..e728e9c2 100644 --- a/src/python/review/quality/rules/function_length_scoring.py +++ b/src/python/review/quality/rules/function_length_scoring.py @@ -13,17 +13,17 @@ class FunctionLengthRuleConfig: LANGUAGE_TO_FUNCTION_LENGTH_RULE_CONFIG = { Language.JAVA: FunctionLengthRuleConfig( - func_len_bad=69 + func_len_bad=69, ), Language.KOTLIN: FunctionLengthRuleConfig( - func_len_bad=69 + func_len_bad=69, ), Language.PYTHON: FunctionLengthRuleConfig( - func_len_bad=49 + func_len_bad=49, ), Language.JS: FunctionLengthRuleConfig( - func_len_bad=69 - ) + func_len_bad=69, + ), } @@ -48,7 +48,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'FunctionLengthRule') -> 'FunctionLengthRule': config = FunctionLengthRuleConfig( - min(self.config.func_len_bad, other.config.func_len_bad) + min(self.config.func_len_bad, other.config.func_len_bad), ) result_rule = FunctionLengthRule(config) result_rule.apply(max(self.func_len, other.func_len)) diff --git a/src/python/review/quality/rules/inheritance_depth_scoring.py b/src/python/review/quality/rules/inheritance_depth_scoring.py index cdabda6b..a3531afc 100644 --- a/src/python/review/quality/rules/inheritance_depth_scoring.py +++ b/src/python/review/quality/rules/inheritance_depth_scoring.py @@ -12,7 +12,7 @@ class InheritanceDepthRuleConfig: common_inheritance_depth_rule_config = InheritanceDepthRuleConfig( - depth_bad=3 + depth_bad=3, ) LANGUAGE_TO_INHERITANCE_DEPTH_RULE_CONFIG = { diff --git a/src/python/review/quality/rules/line_len_scoring.py b/src/python/review/quality/rules/line_len_scoring.py index d52b0382..b188576f 100644 --- a/src/python/review/quality/rules/line_len_scoring.py +++ b/src/python/review/quality/rules/line_len_scoring.py @@ -13,7 +13,7 @@ class LineLengthRuleConfig: common_line_length_rule_config = LineLengthRuleConfig( n_line_len_bad=0.05, - n_line_len_good=0.035 + n_line_len_good=0.035, ) LANGUAGE_TO_LINE_LENGTH_RULE_CONFIG = { @@ -54,7 +54,7 @@ def __get_next_quality_type(self) -> QualityType: def merge(self, other: 'LineLengthRule') -> 'LineLengthRule': config = LineLengthRuleConfig( min(self.config.n_line_len_bad, other.config.n_line_len_bad), - min(self.config.n_line_len_good, other.config.n_line_len_good) + min(self.config.n_line_len_good, other.config.n_line_len_good), ) result_rule = LineLengthRule(config) result_rule.apply(self.n_line_len + other.n_line_len, self.n_lines + other.n_lines) diff --git a/src/python/review/quality/rules/maintainability_scoring.py b/src/python/review/quality/rules/maintainability_scoring.py new file mode 100644 index 00000000..ade6fd4b --- /dev/null +++ b/src/python/review/quality/rules/maintainability_scoring.py @@ -0,0 +1,74 @@ +from dataclasses import dataclass +from typing import Optional + +from src.python.review.common.language import Language +from src.python.review.inspectors.issue import IssueType +from src.python.review.quality.model import QualityType, Rule + + +@dataclass +class MaintainabilityRuleConfig: + maintainability_lack_good: int + maintainability_lack_moderate: int + maintainability_lack_bad: int + + +# TODO: Need testing +# In Radon, the maintainability index is ranked as follows: +# 20-100: Very high +# 10-19: Medium +# 0-9: Extremely low +# Therefore, maintainability_lack_bad = 90, and maintainability_lack_moderate = 80. +common_maintainability_rule_config = MaintainabilityRuleConfig( + maintainability_lack_good=50, + maintainability_lack_moderate=80, + maintainability_lack_bad=90, +) + +LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG = { + Language.JAVA: common_maintainability_rule_config, + Language.PYTHON: common_maintainability_rule_config, + Language.KOTLIN: common_maintainability_rule_config, + Language.JS: common_maintainability_rule_config, +} + + +class MaintainabilityRule(Rule): + def __init__(self, config: MaintainabilityRuleConfig): + self.config = config + self.rule_type = IssueType.MAINTAINABILITY + self.maintainability_lack: Optional[int] = None + + def apply(self, maintainability_lack): + self.maintainability_lack = maintainability_lack + if maintainability_lack > self.config.maintainability_lack_bad: + self.quality_type = QualityType.BAD + self.next_level_delta = maintainability_lack - self.config.maintainability_lack_bad + elif maintainability_lack > self.config.maintainability_lack_moderate: + self.quality_type = QualityType.MODERATE + self.next_level_delta = maintainability_lack - self.config.maintainability_lack_moderate + elif maintainability_lack > self.config.maintainability_lack_good: + self.quality_type = QualityType.GOOD + self.next_level_delta = maintainability_lack - self.config.maintainability_lack_good + else: + self.quality_type = QualityType.EXCELLENT + self.next_level_delta = 0 + self.next_level_type = self.__get_next_quality_type() + + def __get_next_quality_type(self) -> QualityType: + if self.quality_type == QualityType.BAD: + return QualityType.MODERATE + elif self.quality_type == QualityType.MODERATE: + return QualityType.GOOD + return QualityType.EXCELLENT + + def merge(self, other: 'MaintainabilityRule') -> 'MaintainabilityRule': + config = MaintainabilityRuleConfig( + min(self.config.maintainability_lack_bad, other.config.maintainability_lack_bad), + min(self.config.maintainability_lack_moderate, other.config.maintainability_lack_moderate), + min(self.config.maintainability_lack_good, other.config.maintainability_lack_good), + ) + result_rule = MaintainabilityRule(config) + result_rule.apply(max(self.maintainability_lack, other.maintainability_lack)) + + return result_rule diff --git a/src/python/review/quality/rules/method_number_scoring.py b/src/python/review/quality/rules/method_number_scoring.py index 112390c3..bc15a497 100644 --- a/src/python/review/quality/rules/method_number_scoring.py +++ b/src/python/review/quality/rules/method_number_scoring.py @@ -16,7 +16,7 @@ class MethodNumberRuleConfig: common_method_number_rule_config = MethodNumberRuleConfig( method_number_bad=32, method_number_moderate=24, - method_number_good=20 + method_number_good=20, ) LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG = { @@ -60,7 +60,7 @@ def merge(self, other: 'MethodNumberRule') -> 'MethodNumberRule': config = MethodNumberRuleConfig( min(self.config.method_number_bad, other.config.method_number_bad), min(self.config.method_number_moderate, other.config.method_number_moderate), - min(self.config.method_number_good, other.config.method_number_good) + min(self.config.method_number_good, other.config.method_number_good), ) result_rule = MethodNumberRule(config) result_rule.apply(max(self.method_number, other.method_number)) diff --git a/src/python/review/quality/rules/weighted_methods_scoring.py b/src/python/review/quality/rules/weighted_methods_scoring.py index 777d7b2d..633c4839 100644 --- a/src/python/review/quality/rules/weighted_methods_scoring.py +++ b/src/python/review/quality/rules/weighted_methods_scoring.py @@ -16,7 +16,7 @@ class WeightedMethodsRuleConfig: common_weighted_methods_rule_config = WeightedMethodsRuleConfig( weighted_methods_bad=105, weighted_methods_moderate=85, - weighted_methods_good=70 + weighted_methods_good=70, ) LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG = { @@ -58,7 +58,7 @@ def merge(self, other: 'WeightedMethodsRule') -> 'WeightedMethodsRule': config = WeightedMethodsRuleConfig( min(self.config.weighted_methods_bad, other.config.weighted_methods_bad), min(self.config.weighted_methods_moderate, other.config.weighted_methods_moderate), - min(self.config.weighted_methods_good, other.config.weighted_methods_good) + min(self.config.weighted_methods_good, other.config.weighted_methods_good), ) result_rule = WeightedMethodsRule(config) result_rule.apply(max(self.weighted_methods, other.weighted_methods)) diff --git a/src/python/review/reviewers/common.py b/src/python/review/reviewers/common.py index 0d6688c4..9340d824 100644 --- a/src/python/review/reviewers/common.py +++ b/src/python/review/reviewers/common.py @@ -12,6 +12,7 @@ from src.python.review.inspectors.pmd.pmd import PMDInspector from src.python.review.inspectors.pyast.python_ast import PythonAstInspector from src.python.review.inspectors.pylint.pylint import PylintInspector +from src.python.review.inspectors.radon.radon import RadonInspector from src.python.review.quality.evaluate_quality import evaluate_quality from src.python.review.quality.model import Quality from src.python.review.reviewers.review_result import FileReviewResult, ReviewResult @@ -24,6 +25,7 @@ PylintInspector(), Flake8Inspector(), PythonAstInspector(), + RadonInspector(), ], Language.JAVA: [ CheckstyleInspector(), @@ -36,7 +38,7 @@ ], Language.JS: [ ESLintInspector(), - ] + ], } @@ -76,12 +78,12 @@ def perform_language_review(metadata: Metadata, file_review_results.append(FileReviewResult( file_metadata.path, issues, - quality + quality, )) return ReviewResult( file_review_results, - general_quality + general_quality, ) diff --git a/src/python/review/reviewers/perform_review.py b/src/python/review/reviewers/perform_review.py index 277b5187..9f495751 100644 --- a/src/python/review/reviewers/perform_review.py +++ b/src/python/review/reviewers/perform_review.py @@ -14,7 +14,7 @@ from src.python.review.reviewers.utils.print_review import ( print_review_result_as_json, print_review_result_as_multi_file_json, - print_review_result_as_text + print_review_result_as_text, ) logger: Final = logging.getLogger(__name__) @@ -32,7 +32,7 @@ class PathNotExists(Exception): Language.PYTHON: perform_python_review, Language.JAVA: partial(perform_language_review, language=Language.JAVA), Language.KOTLIN: partial(perform_language_review, language=Language.KOTLIN), - Language.JS: partial(perform_language_review, language=Language.JS) + Language.JS: partial(perform_language_review, language=Language.JS), } diff --git a/src/python/review/reviewers/python.py b/src/python/review/reviewers/python.py index a239913a..d51b526a 100644 --- a/src/python/review/reviewers/python.py +++ b/src/python/review/reviewers/python.py @@ -2,7 +2,7 @@ from typing import List from src.python.review.application_config import ApplicationConfig -from src.python.review.common.file_system import get_all_file_system_items, FileSystemItem +from src.python.review.common.file_system import FileSystemItem, get_all_file_system_items from src.python.review.common.language import Language from src.python.review.reviewers.common import perform_language_review from src.python.review.reviewers.review_result import ReviewResult diff --git a/src/python/review/reviewers/utils/code_statistics.py b/src/python/review/reviewers/utils/code_statistics.py index b0523355..19218a7b 100644 --- a/src/python/review/reviewers/utils/code_statistics.py +++ b/src/python/review/reviewers/utils/code_statistics.py @@ -1,7 +1,7 @@ from collections import Counter from dataclasses import dataclass from pathlib import Path -from typing import List, Dict +from typing import Dict, List from src.python.review.common.file_system import get_content_from_file from src.python.review.inspectors.issue import BaseIssue, IssueType @@ -16,6 +16,8 @@ class CodeStatistics: method_number: int max_cyclomatic_complexity: int + max_cohesion_lack: int + max_maintainability_lack: int max_func_len: int max_bool_expr_len: int @@ -38,6 +40,8 @@ def issue_type_to_statistics_dict(self) -> Dict[IssueType, int]: IssueType.METHOD_NUMBER: self.method_number, IssueType.CYCLOMATIC_COMPLEXITY: self.max_cyclomatic_complexity, + IssueType.COHESION: self.max_cohesion_lack, + IssueType.MAINTAINABILITY: self.max_maintainability_lack, IssueType.FUNC_LEN: self.max_func_len, IssueType.BOOL_EXPR_LEN: self.max_bool_expr_len, @@ -45,7 +49,7 @@ def issue_type_to_statistics_dict(self) -> Dict[IssueType, int]: IssueType.INHERITANCE_DEPTH: self.inheritance_depth, IssueType.COUPLING: self.coupling, IssueType.CLASS_RESPONSE: self.class_response, - IssueType.WEIGHTED_METHOD: self.weighted_method_complexities + IssueType.WEIGHTED_METHOD: self.weighted_method_complexities, } @@ -71,7 +75,7 @@ def get_code_style_lines(issues: List[BaseIssue]) -> int: def __get_max_measure_by_issue_type(issue_type: IssueType, issues: List[BaseIssue]) -> int: return max(map( lambda issue: issue.measure(), - filter(lambda issue: issue.type == issue_type, issues) + filter(lambda issue: issue.type == issue_type, issues), ), default=0) @@ -82,6 +86,8 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic bool_expr_lens = __get_max_measure_by_issue_type(IssueType.BOOL_EXPR_LEN, issues) func_lens = __get_max_measure_by_issue_type(IssueType.FUNC_LEN, issues) cyclomatic_complexities = __get_max_measure_by_issue_type(IssueType.CYCLOMATIC_COMPLEXITY, issues) + cohesion_lacks = __get_max_measure_by_issue_type(IssueType.COHESION, issues) + maintainabilities = __get_max_measure_by_issue_type(IssueType.MAINTAINABILITY, issues) # Actually, we expect only one issue with each of the following metrics. inheritance_depths = __get_max_measure_by_issue_type(IssueType.INHERITANCE_DEPTH, issues) @@ -97,6 +103,8 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic max_bool_expr_len=bool_expr_lens, max_func_len=func_lens, n_line_len=issue_type_counter[IssueType.LINE_LEN], + max_cohesion_lack=cohesion_lacks, + max_maintainability_lack=maintainabilities, max_cyclomatic_complexity=cyclomatic_complexities, inheritance_depth=inheritance_depths, class_response=class_responses, @@ -104,5 +112,5 @@ def gather_code_statistics(issues: List[BaseIssue], path: Path) -> CodeStatistic weighted_method_complexities=weighted_method_complexities, method_number=method_numbers, total_lines=__get_total_lines(path), - code_style_lines=get_code_style_lines(issues) + code_style_lines=get_code_style_lines(issues), ) diff --git a/src/python/review/reviewers/utils/issues_filter.py b/src/python/review/reviewers/utils/issues_filter.py index 9ac9b0b6..b6e5529e 100644 --- a/src/python/review/reviewers/utils/issues_filter.py +++ b/src/python/review/reviewers/utils/issues_filter.py @@ -5,10 +5,12 @@ from src.python.review.inspectors.issue import BaseIssue, IssueType, Measurable from src.python.review.quality.rules.boolean_length_scoring import LANGUAGE_TO_BOOLEAN_EXPRESSION_RULE_CONFIG from src.python.review.quality.rules.class_response_scoring import LANGUAGE_TO_RESPONSE_RULE_CONFIG +from src.python.review.quality.rules.cohesion_scoring import LANGUAGE_TO_COHESION_RULE_CONFIG from src.python.review.quality.rules.coupling_scoring import LANGUAGE_TO_COUPLING_RULE_CONFIG from src.python.review.quality.rules.cyclomatic_complexity_scoring import LANGUAGE_TO_CYCLOMATIC_COMPLEXITY_RULE_CONFIG from src.python.review.quality.rules.function_length_scoring import LANGUAGE_TO_FUNCTION_LENGTH_RULE_CONFIG from src.python.review.quality.rules.inheritance_depth_scoring import LANGUAGE_TO_INHERITANCE_DEPTH_RULE_CONFIG +from src.python.review.quality.rules.maintainability_scoring import LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG from src.python.review.quality.rules.method_number_scoring import LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG from src.python.review.quality.rules.weighted_methods_scoring import LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG @@ -22,7 +24,11 @@ def __get_issue_type_to_low_measure_dict(language: Language) -> Dict[IssueType, IssueType.METHOD_NUMBER: LANGUAGE_TO_METHOD_NUMBER_RULE_CONFIG[language].method_number_good, IssueType.COUPLING: LANGUAGE_TO_COUPLING_RULE_CONFIG[language].coupling_moderate, IssueType.CLASS_RESPONSE: LANGUAGE_TO_RESPONSE_RULE_CONFIG[language].response_good, - IssueType.WEIGHTED_METHOD: LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG[language].weighted_methods_good + IssueType.WEIGHTED_METHOD: LANGUAGE_TO_WEIGHTED_METHODS_RULE_CONFIG[language].weighted_methods_good, + IssueType.COHESION: LANGUAGE_TO_COHESION_RULE_CONFIG[language].cohesion_lack_good, + IssueType.MAINTAINABILITY: LANGUAGE_TO_MAINTAINABILITY_RULE_CONFIG[ + language + ].maintainability_lack_good, } @@ -38,7 +44,7 @@ def filter_low_measure_issues(issues: List[BaseIssue], issue_type_to_low_measure_dict = __get_issue_type_to_low_measure_dict(language) # Disable this types of issue, requires further investigation. - ignored_issues = [IssueType.COHESION, IssueType.CHILDREN_NUMBER] + ignored_issues = [IssueType.CHILDREN_NUMBER] return list(filter( lambda issue: issue.type not in ignored_issues and __more_than_low_measure(issue, diff --git a/src/python/review/reviewers/utils/print_review.py b/src/python/review/reviewers/utils/print_review.py index 76c2de04..9da0e35b 100644 --- a/src/python/review/reviewers/utils/print_review.py +++ b/src/python/review/reviewers/utils/print_review.py @@ -24,7 +24,7 @@ def print_review_result_as_text(review_result: ReviewResult, for issue in sorted_issues: line_text = linecache.getline( str(issue.file_path), - issue.line_no + issue.line_no, ).strip() print(f'{issue.line_no} : ' @@ -80,7 +80,7 @@ def print_review_result_as_multi_file_json(review_result: ReviewResult) -> None: 'file_name': str(file_review_result.file_path), 'quality': { 'code': quality_value, - 'text': f'Code quality (beta): {quality_value}' + 'text': f'Code quality (beta): {quality_value}', }, 'issues': [], } @@ -106,7 +106,7 @@ def print_review_result_as_multi_file_json(review_result: ReviewResult) -> None: 'code': quality_value, 'text': f'Code quality (beta): {quality_value}', }, - 'file_review_results': file_review_result_jsons + 'file_review_results': file_review_result_jsons, } print(json.dumps(output_json)) diff --git a/src/python/review/run_tool.py b/src/python/review/run_tool.py index fdbb4b67..9b3b89f8 100644 --- a/src/python/review/run_tool.py +++ b/src/python/review/run_tool.py @@ -1,39 +1,30 @@ import argparse +import enum import logging.config import os import sys import traceback -from enum import Enum, unique from pathlib import Path -from typing import Set, List +from typing import Set + sys.path.append('') sys.path.append('../../..') +from src.python.common.tool_arguments import RunToolArgument, VerbosityLevel from src.python.review.application_config import ApplicationConfig, LanguageVersion from src.python.review.inspectors.inspector_type import InspectorType from src.python.review.logging_config import logging_config -from src.python.review.reviewers.perform_review import OutputFormat, PathNotExists, perform_and_print_review, \ - UnsupportedLanguage +from src.python.review.reviewers.perform_review import ( + OutputFormat, + PathNotExists, + perform_and_print_review, + UnsupportedLanguage, +) logger = logging.getLogger(__name__) -@unique -class VerbosityLevel(Enum): - """ - Same meaning as the logging level. Should be used in command-line args. - """ - DEBUG = '3' - INFO = '2' - ERROR = '1' - DISABLE = '0' - - @classmethod - def values(cls) -> List[str]: - return [member.value for _, member in VerbosityLevel.__members__.items()] - - def parse_disabled_inspectors(value: str) -> Set[InspectorType]: passed_names = value.upper().split(',') allowed_names = {inspector.value for inspector in InspectorType} @@ -51,70 +42,66 @@ def positive_int(value: str) -> int: return value_int -def configure_arguments(parser: argparse.ArgumentParser) -> None: - parser.add_argument('-v', '--verbosity', - help='Choose logging level: ' - f'{VerbosityLevel.ERROR.value} - ERROR; ' - f'{VerbosityLevel.INFO.value} - INFO; ' - f'{VerbosityLevel.DEBUG.value} - DEBUG; ' - f'{VerbosityLevel.DISABLE.value} - disable logging; ' - 'default is 0', +def configure_arguments(parser: argparse.ArgumentParser, tool_arguments: enum.EnumMeta) -> None: + parser.add_argument(tool_arguments.VERBOSITY.value.short_name, + tool_arguments.VERBOSITY.value.long_name, + help=tool_arguments.VERBOSITY.value.description, default=VerbosityLevel.DISABLE.value, choices=VerbosityLevel.values(), - type=str) + type=int) # Usage example: -d Flake8,Intelli - inspectors = [inspector.lower() for inspector in InspectorType.available_values()] - example = f'-d {inspectors[0].lower()},{inspectors[1].lower()}' - - parser.add_argument('-d', '--disable', - help='Disable inspectors. ' - f'Allowed values: {", ".join(inspectors)}. ' - f'Example: {example}', + parser.add_argument(tool_arguments.DISABLE.value.short_name, + tool_arguments.DISABLE.value.long_name, + help=tool_arguments.DISABLE.value.description, type=parse_disabled_inspectors, default=set()) - parser.add_argument('--allow-duplicates', action='store_true', - help='Allow duplicate issues found by different linters. ' - 'By default, duplicates are skipped.') + parser.add_argument(tool_arguments.DUPLICATES.value.long_name, + action='store_true', + help=tool_arguments.DUPLICATES.value.description) # TODO: deprecated argument: language_version. Delete after several releases. - parser.add_argument('--language_version', '--language-version', - help='Specify the language version for JAVA inspectors.', + parser.add_argument('--language_version', + tool_arguments.LANG_VERSION.value.long_name, + help=tool_arguments.LANG_VERSION.value.description, default=None, choices=LanguageVersion.values(), type=str) # TODO: deprecated argument: --n_cpu. Delete after several releases. - parser.add_argument('--n_cpu', '--n-cpu', - help='Specify number of cpu that can be used to run inspectors', + parser.add_argument('--n_cpu', + tool_arguments.CPU.value.long_name, + help=tool_arguments.CPU.value.description, default=1, type=positive_int) - parser.add_argument('path', + parser.add_argument(tool_arguments.PATH.value.long_name, type=lambda value: Path(value).absolute(), - help='Path to file or directory to inspect.') + help=tool_arguments.PATH.value.description) - parser.add_argument('-f', '--format', + parser.add_argument(tool_arguments.FORMAT.value.short_name, + tool_arguments.FORMAT.value.long_name, default=OutputFormat.JSON.value, choices=OutputFormat.values(), type=str, - help='The output format. Default is JSON.') + help=tool_arguments.FORMAT.value.description) - parser.add_argument('-s', '--start-line', + parser.add_argument(tool_arguments.START_LINE.value.short_name, + tool_arguments.START_LINE.value.long_name, default=1, type=positive_int, - help='The first line to be analyzed. It starts from 1.') + help=tool_arguments.START_LINE.value.description) - parser.add_argument('-e', '--end-line', + parser.add_argument(tool_arguments.END_LINE.value.short_name, + tool_arguments.END_LINE.value.long_name, default=None, type=positive_int, - help='The end line to be analyzed or None.') + help=tool_arguments.END_LINE.value.description) - parser.add_argument('--new-format', + parser.add_argument(tool_arguments.NEW_FORMAT.value.long_name, action='store_true', - help='The argument determines whether the tool ' - 'should use the new format') + help=tool_arguments.NEW_FORMAT.value.description) def configure_logging(verbosity: VerbosityLevel) -> None: @@ -132,7 +119,7 @@ def configure_logging(verbosity: VerbosityLevel) -> None: def main() -> int: parser = argparse.ArgumentParser() - configure_arguments(parser) + configure_arguments(parser, RunToolArgument) try: args = parser.parse_args() @@ -150,7 +137,7 @@ def main() -> int: inspectors_config = { 'language_version': LanguageVersion(args.language_version) if args.language_version is not None else None, - 'n_cpu': n_cpu + 'n_cpu': n_cpu, } config = ApplicationConfig( diff --git a/test/python/functional_tests/conftest.py b/test/python/functional_tests/conftest.py index cec7b0a0..da01adde 100644 --- a/test/python/functional_tests/conftest.py +++ b/test/python/functional_tests/conftest.py @@ -1,11 +1,10 @@ from dataclasses import dataclass, field from pathlib import Path +from test.python import TEST_DATA_FOLDER from typing import List, Optional import pytest - from src.python import MAIN_FOLDER -from test.python import TEST_DATA_FOLDER DATA_PATH = TEST_DATA_FOLDER / 'functional_tests' @@ -43,7 +42,7 @@ def build(self) -> List[str]: command.extend([ '--n_cpu', str(self.n_cpu), '-f', self.format, - str(self.path) + str(self.path), ]) if self.start_line is not None: diff --git a/test/python/functional_tests/test_different_languages.py b/test/python/functional_tests/test_different_languages.py index 0cfbf80d..c55e56f9 100644 --- a/test/python/functional_tests/test_different_languages.py +++ b/test/python/functional_tests/test_different_languages.py @@ -1,5 +1,4 @@ import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -10,7 +9,7 @@ def test_python(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -26,7 +25,7 @@ def test_java(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -42,7 +41,7 @@ def test_kotlin(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -59,7 +58,7 @@ def test_all_java_inspectors(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() diff --git a/test/python/functional_tests/test_disable.py b/test/python/functional_tests/test_disable.py index 08655c1e..7967e1bc 100644 --- a/test/python/functional_tests/test_disable.py +++ b/test/python/functional_tests/test_disable.py @@ -1,5 +1,4 @@ import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -10,7 +9,7 @@ def test_disable_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -20,7 +19,7 @@ def test_disable_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() diff --git a/test/python/functional_tests/test_duplicates.py b/test/python/functional_tests/test_duplicates.py index 13ff2a83..9158e030 100644 --- a/test/python/functional_tests/test_duplicates.py +++ b/test/python/functional_tests/test_duplicates.py @@ -1,6 +1,5 @@ import re import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -13,7 +12,7 @@ def test_allow_duplicates(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout_allow_duplicates = process.stdout.decode() @@ -22,7 +21,7 @@ def test_allow_duplicates(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout_filter_duplicates = process.stdout.decode() diff --git a/test/python/functional_tests/test_exit_code.py b/test/python/functional_tests/test_exit_code.py index 70584387..4d5dad1a 100644 --- a/test/python/functional_tests/test_exit_code.py +++ b/test/python/functional_tests/test_exit_code.py @@ -1,6 +1,5 @@ import subprocess from pathlib import Path - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -11,7 +10,7 @@ def test_exit_code_zero(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) assert process.returncode == 0 @@ -24,7 +23,7 @@ def test_exit_code_one(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) assert process.returncode == 1 @@ -37,7 +36,7 @@ def test_exit_code_two(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) assert process.returncode == 2 diff --git a/test/python/functional_tests/test_file_or_project.py b/test/python/functional_tests/test_file_or_project.py index 56bb1251..074c77aa 100644 --- a/test/python/functional_tests/test_file_or_project.py +++ b/test/python/functional_tests/test_file_or_project.py @@ -1,5 +1,4 @@ import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -11,7 +10,7 @@ def test_inspect_file_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -27,7 +26,7 @@ def test_inspect_project_works(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() diff --git a/test/python/functional_tests/test_multi_file_project.py b/test/python/functional_tests/test_multi_file_project.py index db63472f..86a029d1 100644 --- a/test/python/functional_tests/test_multi_file_project.py +++ b/test/python/functional_tests/test_multi_file_project.py @@ -1,35 +1,34 @@ import json import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder EXPECTED_JSON = { 'quality': { 'code': 'EXCELLENT', - 'text': 'Code quality (beta): EXCELLENT' + 'text': 'Code quality (beta): EXCELLENT', }, 'file_review_results': [ { 'file_name': '__init__.py', 'quality': { 'code': 'EXCELLENT', - 'text': 'Code quality (beta): EXCELLENT' + 'text': 'Code quality (beta): EXCELLENT', }, - 'issues': [] + 'issues': [], }, { 'file_name': 'one.py', 'quality': { 'code': 'EXCELLENT', - 'text': 'Code quality (beta): EXCELLENT' + 'text': 'Code quality (beta): EXCELLENT', }, - 'issues': [] + 'issues': [], }, { 'file_name': 'other.py', 'quality': { 'code': 'GOOD', - 'text': 'Code quality (beta): GOOD' + 'text': 'Code quality (beta): GOOD', }, 'issues': [ { @@ -38,7 +37,7 @@ 'line': 'a = 1', 'line_number': 2, 'column_number': 5, - 'category': 'BEST_PRACTICES' + 'category': 'BEST_PRACTICES', }, { 'code': 'W0612', @@ -46,7 +45,7 @@ 'line': 'b = 2', 'line_number': 3, 'column_number': 5, - 'category': 'BEST_PRACTICES' + 'category': 'BEST_PRACTICES', }, { 'code': 'W0612', @@ -54,11 +53,11 @@ 'line': 'c = 3', 'line_number': 4, 'column_number': 5, - 'category': 'BEST_PRACTICES' - } - ] + 'category': 'BEST_PRACTICES', + }, + ], }, - ] + ], } @@ -73,7 +72,7 @@ def test_json_format(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout = process.stdout.decode() print(stdout) diff --git a/test/python/functional_tests/test_range_of_lines.py b/test/python/functional_tests/test_range_of_lines.py index 21710909..1ef4a8f5 100644 --- a/test/python/functional_tests/test_range_of_lines.py +++ b/test/python/functional_tests/test_range_of_lines.py @@ -1,16 +1,15 @@ import json +from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder import pytest - from src.python.review.common.subprocess_runner import run_in_subprocess -from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder PATH_TO_FILE = DATA_PATH / 'lines_range' / 'code_with_multiple_issues.py' EXPECTED_JSON = { 'quality': { 'code': 'BAD', - 'text': 'Code quality (beta): BAD' + 'text': 'Code quality (beta): BAD', }, 'issues': [{ 'category': 'CODE_STYLE', @@ -30,16 +29,16 @@ 'column_number': 2, 'line': 'c=a + b', 'line_number': 4, - 'text': 'missing whitespace around operator' - } - ] + 'text': 'missing whitespace around operator', + }, + ], } NO_ISSUES_JSON = { 'quality': { 'code': 'EXCELLENT', 'text': 'Code quality (beta): EXCELLENT'}, - 'issues': [] + 'issues': [], } @@ -85,8 +84,8 @@ def test_range_filter_when_start_line_is_not_first( 'line': 'c=a + b', 'line_number': 4, 'column_number': 2, - 'category': 'CODE_STYLE' - }] + 'category': 'CODE_STYLE', + }], } assert output_json == expected_json_with_one_issue @@ -145,7 +144,7 @@ def test_range_filter_when_end_line_is_first( expected_json_with_one_issue = { 'quality': { 'code': 'MODERATE', - 'text': 'Code quality (beta): MODERATE' + 'text': 'Code quality (beta): MODERATE', }, 'issues': [{ 'code': 'E225', @@ -153,8 +152,8 @@ def test_range_filter_when_end_line_is_first( 'line': 'a=10', 'line_number': 1, 'column_number': 2, - 'category': 'CODE_STYLE' - }] + 'category': 'CODE_STYLE', + }], } assert output_json == expected_json_with_one_issue @@ -211,7 +210,7 @@ def test_range_filter_when_both_start_and_end_lines_specified_not_equal_borders( expected_json = { 'quality': { 'code': 'BAD', - 'text': 'Code quality (beta): BAD' + 'text': 'Code quality (beta): BAD', }, 'issues': [{ 'code': 'E225', @@ -219,15 +218,15 @@ def test_range_filter_when_both_start_and_end_lines_specified_not_equal_borders( 'line': 'b=20', 'line_number': 2, 'column_number': 2, - 'category': 'CODE_STYLE' + 'category': 'CODE_STYLE', }, { 'code': 'E225', 'text': 'missing whitespace around operator', 'line': 'c=a + b', 'line_number': 4, 'column_number': 2, - 'category': 'CODE_STYLE' - }] + 'category': 'CODE_STYLE', + }], } assert output_json == expected_json diff --git a/test/python/functional_tests/test_single_file_json_format.py b/test/python/functional_tests/test_single_file_json_format.py index d71c115b..85616145 100644 --- a/test/python/functional_tests/test_single_file_json_format.py +++ b/test/python/functional_tests/test_single_file_json_format.py @@ -1,10 +1,9 @@ import json import subprocess +from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder from jsonschema import validate -from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder - schema = { 'type': 'object', 'properties': { @@ -12,8 +11,8 @@ 'type': 'object', 'properties': { 'code': {'type': 'string'}, - 'text': {'type': 'string'} - } + 'text': {'type': 'string'}, + }, }, 'issues': { 'type': 'array', @@ -25,11 +24,11 @@ 'column_number': {'type': 'number'}, 'line': {'type': 'string'}, 'line_number': {'type': 'number'}, - 'text': {'type': 'string'} - } - } - } - } + 'text': {'type': 'string'}, + }, + }, + }, + }, } @@ -43,7 +42,7 @@ def test_json_format(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) stdout = process.stdout.decode() diff --git a/test/python/functional_tests/test_verbosity.py b/test/python/functional_tests/test_verbosity.py index 67df44eb..e835258b 100644 --- a/test/python/functional_tests/test_verbosity.py +++ b/test/python/functional_tests/test_verbosity.py @@ -1,6 +1,5 @@ import json import subprocess - from test.python.functional_tests.conftest import DATA_PATH, LocalCommandBuilder @@ -13,7 +12,7 @@ def test_disable_logs_text(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() output = output.lower() @@ -33,7 +32,7 @@ def test_disable_logs_json(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() @@ -49,7 +48,7 @@ def test_enable_all_logs(local_command: LocalCommandBuilder): process = subprocess.run( local_command.build(), stdout=subprocess.PIPE, - stderr=subprocess.PIPE + stderr=subprocess.PIPE, ) output = process.stdout.decode() output = output.lower() diff --git a/test/python/inspectors/conftest.py b/test/python/inspectors/conftest.py index 615f95dc..f9072db7 100644 --- a/test/python/inspectors/conftest.py +++ b/test/python/inspectors/conftest.py @@ -5,7 +5,6 @@ from typing import Any, Dict, List import pytest - from src.python.review.common.file_system import new_temp_dir from src.python.review.inspectors.issue import BaseIssue, IssueType from src.python.review.reviewers.utils.metadata_exploration import explore_file, FileMetadata @@ -31,16 +30,16 @@ def branch_info_response() -> Dict[str, Any]: 'reachability': 1, }, 'canCreateReview': { - 'isAllowed': True + 'isAllowed': True, }, 'stats': { 'parentBranch': 'bar', 'commitsAhead': 0, - 'commitsBehind': 0 + 'commitsBehind': 0, }, 'mergeInfo': {}, - 'isPullRequest': False - } + 'isPullRequest': False, + }, } @@ -52,15 +51,15 @@ def ownership_summary_response() -> Dict[str, Any]: { 'filePath': '/foo.py', 'state': 0, - 'userId': None + 'userId': None, }, { 'filePath': '/bar/baz.py', 'state': 0, - 'userId': None - } - ] - } + 'userId': None, + }, + ], + }, } @@ -73,6 +72,8 @@ class IssuesTestInfo: n_cc: int = 0 n_bool_expr_len: int = 0 n_other_complexity: int = 0 + n_cohesion: int = 0 + n_maintainability: int = 0 def gather_issues_test_info(issues: List[BaseIssue]) -> IssuesTestInfo: @@ -85,7 +86,9 @@ def gather_issues_test_info(issues: List[BaseIssue]) -> IssuesTestInfo: n_func_len=counter[IssueType.FUNC_LEN], n_cc=counter[IssueType.CYCLOMATIC_COMPLEXITY], n_bool_expr_len=counter[IssueType.BOOL_EXPR_LEN], - n_other_complexity=counter[IssueType.COMPLEXITY] + n_other_complexity=counter[IssueType.COMPLEXITY], + n_cohesion=counter[IssueType.COHESION], + n_maintainability=counter[IssueType.MAINTAINABILITY], ) diff --git a/test/python/inspectors/test_checkstyle_inspector.py b/test/python/inspectors/test_checkstyle_inspector.py index 892ec97f..ac691ff6 100644 --- a/test/python/inspectors/test_checkstyle_inspector.py +++ b/test/python/inspectors/test_checkstyle_inspector.py @@ -1,10 +1,10 @@ -import pytest +from test.python.inspectors import JAVA_DATA_FOLDER +from test.python.inspectors.conftest import gather_issues_test_info, IssuesTestInfo, use_file_metadata +import pytest from src.python.review.common.language import Language from src.python.review.inspectors.checkstyle.checkstyle import CheckstyleInspector from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues -from test.python.inspectors import JAVA_DATA_FOLDER -from test.python.inspectors.conftest import gather_issues_test_info, IssuesTestInfo, use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('test_simple_valid_program.java', 0), @@ -43,6 +43,7 @@ ('test_indentation_with_spaces.java', 0), ('test_indentation_with_tabs.java', 0), ('test_indentation_google_style.java', 4), + ('test_multiple_literals.java', 1), ] diff --git a/test/python/inspectors/test_detekt_inspector.py b/test/python/inspectors/test_detekt_inspector.py index 4dbafd2d..99f84f27 100644 --- a/test/python/inspectors/test_detekt_inspector.py +++ b/test/python/inspectors/test_detekt_inspector.py @@ -1,10 +1,10 @@ -import pytest +from test.python.inspectors import KOTLIN_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata +import pytest from src.python.review.common.language import Language from src.python.review.inspectors.detekt.detekt import DetektInspector from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues -from test.python.inspectors import KOTLIN_DATA_FOLDER -from test.python.inspectors.conftest import use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('case0_good_program.kt', 0), @@ -23,6 +23,7 @@ ('case16_redundant_unit.kt', 1), ('case18_redundant_braces.kt', 3), ('case20_cyclomatic_complexity.kt', 0), + ('case21_cyclomatic_complexity_bad.kt', 2), ('case22_too_many_arguments.kt', 1), ('case23_bad_range_performance.kt', 3), ('case24_duplicate_when_bug.kt', 1), diff --git a/test/python/inspectors/test_eslint_inspector.py b/test/python/inspectors/test_eslint_inspector.py index f6681c90..d7aceeac 100644 --- a/test/python/inspectors/test_eslint_inspector.py +++ b/test/python/inspectors/test_eslint_inspector.py @@ -1,10 +1,10 @@ -import pytest +from test.python.inspectors import JS_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata +import pytest from src.python.review.common.language import Language from src.python.review.inspectors.eslint.eslint import ESLintInspector from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues -from test.python.inspectors import JS_DATA_FOLDER -from test.python.inspectors.conftest import use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('case0_no_issues.js', 0), diff --git a/test/python/inspectors/test_flake8_inspector.py b/test/python/inspectors/test_flake8_inspector.py index 3c655331..2a2ebf26 100644 --- a/test/python/inspectors/test_flake8_inspector.py +++ b/test/python/inspectors/test_flake8_inspector.py @@ -1,25 +1,26 @@ -import pytest +from test.python.inspectors import PYTHON_DATA_FOLDER +from test.python.inspectors.conftest import gather_issues_test_info, IssuesTestInfo, use_file_metadata +from textwrap import dedent +import pytest from src.python.review.common.language import Language from src.python.review.inspectors.flake8.flake8 import Flake8Inspector from src.python.review.inspectors.issue import IssueType from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues -from test.python.inspectors import PYTHON_DATA_FOLDER -from test.python.inspectors.conftest import gather_issues_test_info, IssuesTestInfo, use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('case0_spaces.py', 5), ('case1_simple_valid_program.py', 0), - ('case2_boolean_expressions.py', 1), - ('case3_redefining_builtin.py', 1), - ('case4_naming.py', 10), + ('case2_boolean_expressions.py', 8), + ('case3_redefining_builtin.py', 2), + ('case4_naming.py', 8), ('case5_returns.py', 1), ('case6_unused_variables.py', 3), ('case8_good_class.py', 0), - ('case7_empty_lines.py', 0), + ('case7_empty_lines.py', 2), ('case10_unused_variable_in_loop.py', 1), - ('case13_complex_logic.py', 3), - ('case13_complex_logic_2.py', 1), + ('case13_complex_logic.py', 12), + ('case13_complex_logic_2.py', 3), ('case11_redundant_parentheses.py', 0), ('case14_returns_errors.py', 4), ('case16_comments.py', 0), @@ -29,6 +30,12 @@ ('case21_imports.py', 2), ('case25_django.py', 0), ('case31_spellcheck.py', 0), + ('case32_string_format.py', 34), + ('case33_commas.py', 14), + ('case34_cohesion.py', 1), + ('case35_line_break.py', 11), + ('case36_unpacking.py', 3), + ('case37_wildcard_import.py', 7), ] @@ -39,7 +46,7 @@ def test_file_with_issues(file_name: str, n_issues: int): path_to_file = PYTHON_DATA_FOLDER / file_name with use_file_metadata(path_to_file) as file_metadata: issues = inspector.inspect(file_metadata.path, {}) - issues = filter_low_measure_issues(issues, Language.PYTHON) + issues = list(filter(lambda i: i.type != IssueType.INFO, filter_low_measure_issues(issues, Language.PYTHON))) assert len(issues) == n_issues @@ -48,20 +55,31 @@ def test_file_with_issues(file_name: str, n_issues: int): ('case0_spaces.py', IssuesTestInfo(n_code_style=5)), ('case1_simple_valid_program.py', IssuesTestInfo()), ('case2_boolean_expressions.py', IssuesTestInfo(n_code_style=1, - n_cc=8)), - ('case3_redefining_builtin.py', IssuesTestInfo(n_error_prone=1)), - ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_best_practices=0, n_cc=5)), + n_best_practices=4, + n_error_prone=1, + n_cc=8, + n_other_complexity=2)), + ('case3_redefining_builtin.py', IssuesTestInfo(n_error_prone=2)), + ('case4_naming.py', IssuesTestInfo(n_code_style=7, n_cc=5, n_cohesion=1)), ('case6_unused_variables.py', IssuesTestInfo(n_best_practices=3, n_cc=1)), - ('case8_good_class.py', IssuesTestInfo(n_cc=1)), - ('case7_empty_lines.py', IssuesTestInfo(n_cc=4)), + ('case8_good_class.py', IssuesTestInfo(n_cc=1, n_cohesion=1)), + ('case7_empty_lines.py', IssuesTestInfo(n_cc=5, n_cohesion=2)), ('case10_unused_variable_in_loop.py', IssuesTestInfo(n_best_practices=1, n_cc=1)), - ('case13_complex_logic.py', IssuesTestInfo(n_cc=6)), - ('case13_complex_logic_2.py', IssuesTestInfo(n_cc=2)), + ('case13_complex_logic.py', IssuesTestInfo(n_cc=5, n_other_complexity=10)), + ('case13_complex_logic_2.py', IssuesTestInfo(n_cc=2, n_other_complexity=2)), ('case14_returns_errors.py', IssuesTestInfo(n_best_practices=1, n_error_prone=3, n_cc=4)), + ('case35_line_break.py', IssuesTestInfo(n_best_practices=1, + n_code_style=10, + n_cc=1)), + ('case32_string_format.py', IssuesTestInfo(n_error_prone=28, n_other_complexity=6)), + ('case33_commas.py', IssuesTestInfo(n_code_style=14, n_cc=4)), + ('case34_cohesion.py', IssuesTestInfo(n_cc=6, n_cohesion=2)), + ('case36_unpacking.py', IssuesTestInfo(n_error_prone=2, n_cc=1, n_other_complexity=1)), + ('case37_wildcard_import.py', IssuesTestInfo(n_best_practices=1, n_error_prone=3, n_cc=2, n_other_complexity=2)), ] @@ -79,9 +97,12 @@ def test_file_with_issues_info(file_name: str, expected_issues_info: IssuesTestI def test_parse(): file_name = 'test.py' - output = ('test.py:1:11:W602:test 1\n' - 'test.py:2:12:E703:test 2\n' - 'test.py:3:13:SC200:test 3') + output = f"""\ + {file_name}:1:11:W602:test 1 + {file_name}:2:12:E703:test 2 + {file_name}:3:13:SC200:test 3 + """ + output = dedent(output) issues = Flake8Inspector.parse(output) @@ -99,7 +120,7 @@ def test_choose_issue_type(): expected_issue_types = [ IssueType.ERROR_PRONE, IssueType.INFO, IssueType.ERROR_PRONE, IssueType.BEST_PRACTICES, - IssueType.CODE_STYLE + IssueType.CODE_STYLE, ] issue_types = list(map(Flake8Inspector.choose_issue_type, error_codes)) diff --git a/test/python/inspectors/test_local_review.py b/test/python/inspectors/test_local_review.py index 8f9491a0..5b182540 100644 --- a/test/python/inspectors/test_local_review.py +++ b/test/python/inspectors/test_local_review.py @@ -1,20 +1,19 @@ import json from collections import namedtuple +from test.python.inspectors import PYTHON_DATA_FOLDER import pytest - from src.python.review.application_config import ApplicationConfig from src.python.review.inspectors.inspector_type import InspectorType from src.python.review.quality.model import QualityType from src.python.review.reviewers.perform_review import OutputFormat, PathNotExists, perform_and_print_review -from test.python.inspectors import PYTHON_DATA_FOLDER Args = namedtuple('Args', [ 'path', 'allow_duplicates', 'disable', 'format', - 'handler' + 'handler', ]) @@ -24,7 +23,7 @@ def config() -> ApplicationConfig: disabled_inspectors={InspectorType.INTELLIJ}, allow_duplicates=False, n_cpu=1, - inspectors_config={'n_cpu': 1} + inspectors_config={"n_cpu": 1}, ) diff --git a/test/python/inspectors/test_out_of_range_issues.py b/test/python/inspectors/test_out_of_range_issues.py index 016928b7..3f73d63d 100644 --- a/test/python/inspectors/test_out_of_range_issues.py +++ b/test/python/inspectors/test_out_of_range_issues.py @@ -44,7 +44,7 @@ def test_out_of_range_issues_when_the_same_borders() -> None: first_line_issues = [ create_code_issue_by_line(1), create_code_issue_by_line(1), - create_code_issue_by_line(1) + create_code_issue_by_line(1), ] assert filter_out_of_range_issues(first_line_issues, diff --git a/test/python/inspectors/test_pmd_inspector.py b/test/python/inspectors/test_pmd_inspector.py index 18b13af4..3702a79b 100644 --- a/test/python/inspectors/test_pmd_inspector.py +++ b/test/python/inspectors/test_pmd_inspector.py @@ -1,7 +1,8 @@ -import pytest +from test.python.inspectors import JAVA_DATA_FOLDER +import pytest from src.python.review.inspectors.pmd.pmd import PMDInspector -from test.python.inspectors import JAVA_DATA_FOLDER + from .conftest import use_file_metadata FILE_NAMES_AND_N_ISSUES = [ @@ -14,7 +15,7 @@ ('test_comparing_strings.java', 3), ('test_constants.java', 4), ('test_covariant_equals.java', 1), - ('test_curly_braces.java', 2), + ('test_curly_braces.java', 0), ('test_double_checked_locking.java', 2), ('test_for_loop.java', 2), ('test_implementation_types.java', 0), @@ -31,6 +32,7 @@ ('test_valid_curly_braces.java', 0), ('test_when_only_equals_overridden.java', 1), ('test_valid_spaces.java', 0), + ('test_multiple_literals.java', 1), ] diff --git a/test/python/inspectors/test_pylint_inspector.py b/test/python/inspectors/test_pylint_inspector.py index e1da830a..fc381234 100644 --- a/test/python/inspectors/test_pylint_inspector.py +++ b/test/python/inspectors/test_pylint_inspector.py @@ -1,15 +1,17 @@ -import pytest +import textwrap +from test.python.inspectors import PYTHON_DATA_FOLDER +import pytest from src.python.review.inspectors.issue import IssueType from src.python.review.inspectors.pylint.pylint import PylintInspector -from test.python.inspectors import PYTHON_DATA_FOLDER + from .conftest import use_file_metadata FILE_NAMES_AND_N_ISSUES = [ ('case0_spaces.py', 0), ('case1_simple_valid_program.py', 0), ('case2_boolean_expressions.py', 3), - ('case3_redefining_builtin.py', 1), + ('case3_redefining_builtin.py', 0), ('case4_naming.py', 3), ('case5_returns.py', 1), ('case6_unused_variables.py', 4), @@ -30,6 +32,8 @@ ('case25_django.py', 0), ('case27_using_requests.py', 0), ('case30_allow_else_return.py', 0), + ('case36_unpacking.py', 0), + ('case37_wildcard_import.py', 1), ] @@ -46,8 +50,11 @@ def test_file_with_issues(file_name: str, n_issues: int): def test_parse(): file_name = 'test.py' - output = 'test.py:1:11:R0123:test 1\n' \ - 'test.py:2:12:C1444:test 2' + output = f"""\ + {file_name}:1:11:R0123:test 1 + {file_name}:2:12:C1444:test 2 + """ + output = textwrap.dedent(output) issues = PylintInspector.parse(output) @@ -67,7 +74,7 @@ def test_choose_issue_type(): IssueType.BEST_PRACTICES, IssueType.BEST_PRACTICES, IssueType.CODE_STYLE, - IssueType.ERROR_PRONE + IssueType.ERROR_PRONE, ] issue_types = list( diff --git a/test/python/inspectors/test_python_ast.py b/test/python/inspectors/test_python_ast.py index 09192cd3..880e4775 100644 --- a/test/python/inspectors/test_python_ast.py +++ b/test/python/inspectors/test_python_ast.py @@ -1,12 +1,14 @@ import ast +from test.python.inspectors import PYTHON_AST_DATA_FOLDER, PYTHON_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata import pytest - from src.python.review.inspectors.inspector_type import InspectorType -from src.python.review.inspectors.pyast.python_ast import BoolExpressionLensGatherer, FunctionLensGatherer, \ - PythonAstInspector -from test.python.inspectors import PYTHON_DATA_FOLDER, PYTHON_AST_DATA_FOLDER -from test.python.inspectors.conftest import use_file_metadata +from src.python.review.inspectors.pyast.python_ast import ( + BoolExpressionLensGatherer, + FunctionLensGatherer, + PythonAstInspector, +) FILE_NAMES_AND_N_ISSUES = [ ('case0_spaces.py', 0), diff --git a/test/python/inspectors/test_radon_inspector.py b/test/python/inspectors/test_radon_inspector.py new file mode 100644 index 00000000..4ed41e1e --- /dev/null +++ b/test/python/inspectors/test_radon_inspector.py @@ -0,0 +1,52 @@ +from test.python.inspectors import PYTHON_DATA_FOLDER +from test.python.inspectors.conftest import use_file_metadata +from textwrap import dedent + +import pytest +from src.python.review.common.language import Language +from src.python.review.inspectors.issue import IssueType +from src.python.review.inspectors.radon.radon import RadonInspector +from src.python.review.inspectors.tips import get_maintainability_index_tip +from src.python.review.reviewers.utils.issues_filter import filter_low_measure_issues + + +FILE_NAMES_AND_N_ISSUES = [ + ("case13_complex_logic.py", 1), + ("case13_complex_logic_2.py", 1), + ("case8_good_class.py", 0), +] + + +@pytest.mark.parametrize(("file_name", "n_issues"), FILE_NAMES_AND_N_ISSUES) +def test_file_with_issues(file_name: str, n_issues: int): + inspector = RadonInspector() + + path_to_file = PYTHON_DATA_FOLDER / file_name + with use_file_metadata(path_to_file) as file_metadata: + issues = inspector.inspect(file_metadata.path, {}) + issues = filter_low_measure_issues(issues, Language.PYTHON) + + assert len(issues) == n_issues + + +def test_mi_parse(): + file_name = "test.py" + output = f"""\ + {file_name} - C (4.32) + {file_name} - B (13.7) + {file_name} - A (70.0) + """ + output = dedent(output) + + issues = RadonInspector.mi_parse(output) + + assert all(str(issue.file_path) == file_name for issue in issues) + assert [issue.line_no for issue in issues] == [1, 1, 1] + assert [issue.column_no for issue in issues] == [1, 1, 1] + assert [issue.description for issue in issues] == [get_maintainability_index_tip()] * 3 + assert [issue.type for issue in issues] == [ + IssueType.MAINTAINABILITY, + IssueType.MAINTAINABILITY, + IssueType.MAINTAINABILITY, + ] + assert [issue.maintainability_lack for issue in issues] == [95, 86, 30] diff --git a/test/python/inspectors/test_springlint_inspector.py b/test/python/inspectors/test_springlint_inspector.py index 18ee8dfb..673aa30c 100644 --- a/test/python/inspectors/test_springlint_inspector.py +++ b/test/python/inspectors/test_springlint_inspector.py @@ -1,6 +1,7 @@ +from test.python.inspectors import SPRING_DATA_FOLDER + from src.python.review.inspectors.issue import IssueType from src.python.review.inspectors.springlint.springlint import SpringlintInspector -from test.python.inspectors import SPRING_DATA_FOLDER def test_controller_with_smells(): diff --git a/test/resources/inspectors/java/test_multiple_literals.java b/test/resources/inspectors/java/test_multiple_literals.java new file mode 100644 index 00000000..58a9e2ee --- /dev/null +++ b/test/resources/inspectors/java/test_multiple_literals.java @@ -0,0 +1,32 @@ +class Main { + public static void main(String[] args) { + // ok + String shortRareLiteral1 = "12"; + String shortRareLiteral2 = "12"; + String shortRareLiteral3 = "12"; + + // ok + String longRareLiteral1 = "123"; + String longRareLiteral2 = "123"; + String longRareLiteral3 = "123"; + + // ok + String shortFrequentLiteral1 = "34"; + String shortFrequentLiteral2 = "34"; + String shortFrequentLiteral3 = "34"; + String shortFrequentLiteral4 = "34"; + + // warning + String longFrequentLiteral1 = "456"; + String longFrequentLiteral2 = "456"; + String longFrequentLiteral3 = "456"; + String longFrequentLiteral4 = "456"; + + System.out.println( + shortRareLiteral1 + shortRareLiteral2 + shortRareLiteral3 + + longRareLiteral1 + longRareLiteral2 + longRareLiteral3 + + shortFrequentLiteral1 + shortFrequentLiteral2 + shortFrequentLiteral3 + shortFrequentLiteral4 + + longFrequentLiteral1 + longFrequentLiteral2 + longFrequentLiteral3 + longFrequentLiteral4 + ); + } +} diff --git a/test/resources/inspectors/python/case13_complex_logic.py b/test/resources/inspectors/python/case13_complex_logic.py index 60718a89..3208a9bf 100644 --- a/test/resources/inspectors/python/case13_complex_logic.py +++ b/test/resources/inspectors/python/case13_complex_logic.py @@ -8,16 +8,22 @@ def max_of_three(a, b, c): return a +FEW_UNITS_NUMBER = 9 +PACK_UNITS_NUMBER = 49 +HORDE_UNITS_NUMBER = 499 +SWARM_UNITS_NUMBER = 999 + + def army_of_units(count): if count < 1: print("no army") - elif count <= 9: + elif count <= FEW_UNITS_NUMBER: print('few') - elif count <= 49: + elif count <= PACK_UNITS_NUMBER: print('pack') - elif count <= 499: + elif count <= HORDE_UNITS_NUMBER: print("horde") - elif count <= 999: + elif count <= SWARM_UNITS_NUMBER: print('swarm') else: print('legion') @@ -65,60 +71,38 @@ def determine_strange_quark(spin, charge): print("Higgs boson Boson") +SHEEP_PRICE = 6769 +COW_PRICE = 3848 +PIG_PRICE = 1296 +GOAT_PRICE = 678 +DOG_PRICE = 137 +CHICKEN_PRICE = 23 + + def buy_animal(money): - if money >= 6769: - print(str(money // 6769) + " sheep") - elif money >= 3848: + if money >= SHEEP_PRICE: + number_of_sheep = money // SHEEP_PRICE + print(f"{number_of_sheep} sheep") + elif money >= COW_PRICE: print("1 cow") - elif money >= 1296: - if money // 1296 == 1: + elif money >= PIG_PRICE: + if money // PIG_PRICE == 1: print("1 pig") else: print("2 pigs") - elif money >= 678: + elif money >= GOAT_PRICE: print("1 goat") - elif money >= 137: - if money // 137 == 1: + elif money >= DOG_PRICE: + if money // DOG_PRICE == 1: print("1 dog") else: - print(str(money // 137) + " dogs") - elif money >= 23: - if money // 23 == 1: + number_of_dogs = money // DOG_PRICE + print(f"{number_of_dogs} dogs") + elif money >= CHICKEN_PRICE: + if money // CHICKEN_PRICE == 1: print("1 chicken") else: - print(str(money // 23) + " chickens") + number_of_chickens = money // CHICKEN_PRICE + print(f"{number_of_chickens} chickens") else: print("None") - - -def fun_with_complex_logic(a, b, c): - d = 0 - if a > 10: - d = 30 - elif a < 100: - d = 50 - elif a == 300 and b == 40: - for i in range(9): - a += i - elif a == 200: - if b > 300 and c < 50: - d = 400 - else: - d = 800 - elif a == 2400: - if b > 500 and c < 50: - d = 400 - else: - d = 800 - elif a == 1000: - if b == 900: - if c == 1000: - d = 10000 - else: - d = 900 - elif c == 300: - d = 1000 - elif a + b == 400: - d = 400 - print(d) - return d diff --git a/test/resources/inspectors/python/case13_complex_logic_2.py b/test/resources/inspectors/python/case13_complex_logic_2.py index 1eaf13dc..8f670bc1 100644 --- a/test/resources/inspectors/python/case13_complex_logic_2.py +++ b/test/resources/inspectors/python/case13_complex_logic_2.py @@ -1,10 +1,14 @@ elements = list(input('Enter cells: ')) y = 0 o = 0 + +CROSS_SYMBOL = 'X' +NOUGHT_SYMBOL = 'O' + for x in elements: - if x == 'X': + if x == CROSS_SYMBOL: y = y + 1 - elif x == 'O': + elif x == NOUGHT_SYMBOL: o = o + 1 odds = abs(y - o) @@ -22,8 +26,8 @@ full_field = [up_row, up_col, mid_row, mid_col, down_row, down_col, diagonal_1, diagonal_2] -x_win = ['X', 'X', 'X'] -o_win = ['O', 'O', 'O'] +x_win = [CROSS_SYMBOL, CROSS_SYMBOL, CROSS_SYMBOL] +o_win = [NOUGHT_SYMBOL, NOUGHT_SYMBOL, NOUGHT_SYMBOL] field = f""" --------- @@ -35,10 +39,10 @@ if odds < 2: if x_win in full_field and o_win not in full_field: print(field) - print('X wins') + print(f'{CROSS_SYMBOL} wins') elif o_win in full_field and x_win not in full_field: print(field) - print('O wins') + print(f'{NOUGHT_SYMBOL} wins') elif o_win in full_field and x_win in full_field: print(field) print('Impossible') diff --git a/test/resources/inspectors/python/case18_comprehensions.py b/test/resources/inspectors/python/case18_comprehensions.py index 3ed9be6f..4365018e 100644 --- a/test/resources/inspectors/python/case18_comprehensions.py +++ b/test/resources/inspectors/python/case18_comprehensions.py @@ -11,7 +11,7 @@ dict(((1, 2),)) # {1: 2} -sum([x ** 2 for x in range(10)]) # sum(x ** 2 for x in range(10)) +sum([x ** 2 for x in range(10)]) # we allow this since flake8-comprehension 3.4.0 (see its changelog) test_dict = dict() # we allow this test_list = list() # we allow this diff --git a/test/resources/inspectors/python/case32_string_format.py b/test/resources/inspectors/python/case32_string_format.py new file mode 100644 index 00000000..bee24195 --- /dev/null +++ b/test/resources/inspectors/python/case32_string_format.py @@ -0,0 +1,143 @@ +hello_world = "Hello, World!" +hello = "Hello" +world = "World" +some_list = [hello, world] +some_dict = {"hello": hello, "world": world} + +# ----------------------------------------------------------------------------------- + +# Correct +print("{0}!".format(hello_world)) + +# Correct +print("{}!".format(hello_world)) + +# Correct +print("{0}, {1}!".format(hello, world)) + +# Correct +print("{0}, {1}!".format(*some_list)) + +# Correct +print("{1}, {0}!".format(world, hello)) + +# Correct +print("{1}, {0}!".format(*some_list)) + +# Correct +print("{}, {}!".format(hello, world)) + +# Correct +print("{}, {}!".format(*some_list)) + +# Correct +print("{hello}, {world}!".format(hello=hello, world=world)) + +# Correct +print("{hello}, {world}!".format(**some_dict)) + +# ----------------------------------------------------------------------------------- + +# Correct +print(str.format("{0}!", hello_world)) + +# Correct +print(str.format("{}!", hello_world)) + +# Correct +print(str.format("{0}, {1}!", hello, world)) + +# Correct +print(str.format("{0}, {1}!", *some_list)) + +# Correct +print(str.format("{1}, {0}!", world, hello)) + +# Correct +print(str.format("{1}, {0}!", *some_list)) + +# Correct +print(str.format("{}, {}!", hello, world)) + +# Correct +print(str.format("{}, {}!", *some_list)) + +# Correct +print(str.format("{hello}, {world}!", hello=hello, world=world)) + +# Correct +print(str.format("{hello}, {world}!", **some_dict)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-201 +print("{0}, {1}!".format(hello)) + +# Wrong: P-201 +print("{}, {}!".format(hello)) + +# Wrong: P-201 +print(str.format("{0}, {1}!", hello)) + +# Wrong: P-201 +print(str.format("{}, {}!", hello)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-202 +print("{hello}, {world}!".format(hello=hello)) + +# Wrong: P-202 +print(str.format("{hello}, {world}!", hello=hello)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-203 +print("{0}!".format(**some_dict)) + +# Wrong: P-203 +print("{}!".format(**some_dict)) + +# Wrong: P-203 +print(str.format("{0}!", **some_dict)) + +# Wrong: P-203 +print(str.format("{}!", **some_dict)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-204 +print("{hello_world}!".format(*some_list)) + +# Wrong: P-204 +print(str.format("{hello_world}!", *some_list)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-205 +print("{}, {0}!".format(hello, world)) + +# Wrong: P-205 +print(str.format("{}, {0}!", hello, world)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-301 +print("{0}".format(hello_world, hello, world)) + +# Wrong: P-301 +print("{}".format(hello_world, hello, world)) + +# Wrong: P-301 +print(str.format("{0}", hello_world, hello, world)) + +# Wrong: P-301 +print(str.format("{}", hello_world, hello, world)) + +# ----------------------------------------------------------------------------------- + +# Wrong: P-302 +print("{hello_world}".format(hello_world=hello_world, hello=hello, world=world)) + +# Wrong: P-302 +print(str.format("{hello_world}", hello_world=hello_world, hello=hello, world=world)) diff --git a/test/resources/inspectors/python/case33_commas.py b/test/resources/inspectors/python/case33_commas.py new file mode 100644 index 00000000..6af55963 --- /dev/null +++ b/test/resources/inspectors/python/case33_commas.py @@ -0,0 +1,153 @@ +# Wrong C-812 +from math import ( + log +) + +# Correct +from math import ( + sin, +) + +# Wrong C-812 +bad_multiline_dict = { + "first": 1, + "second": 2 +} + +# Correct +good_multiline_dict = { + "first": 1, + "second": 2, +} + +# Wrong C-812 +bad_multiline_list = [ + 1, + 2, + 3 +] + +# Correct +good_multiline_list = [ + 1, + 2, + 3, +] + +# Wrong C-812 +bad_multiline_tuple = ( + 3, + 4 +) + +good_multiline_tuple = ( + 3, + 4, +) + + +# Wrong C-812 +def bad_function( + a, + b +): + return log(a, b) + + +bad_function( + 1, + 2 +) + +bad_function( + a=1, + b=2 +) + + +# Correct +def good_function( + a, + b, +): + return a + sin(b) + + +good_function( + 1, + 2, +) + +good_function( + a=1, + b=2, +) + +# Wrong: C-813 +print( + "Hello", + "World" +) + +# Correct +print( + "Hello", + "World", +) + + +# Wrong: C-816 +def bad_function_with_unpacking( + a, + b, + **kwargs +): + pass + + +# Correct +def good_function_with_unpacking( + a, + b, + **kwargs, +): + pass + + +# Wrong: C-815 +good_function_with_unpacking( + 1, + 2, + **good_multiline_dict +) + +# Correct +good_function_with_unpacking( + 1, + 2, + **good_multiline_dict, +) + +# Wrong: C-818 +bad_comma = 1, + +# Correct +good_comma = (1,) + +# Wrong: C-819 +bad_list = [1, 2, 3, ] + +# Correct: +good_list = [1, 2, 3] + +# Wrong: C-819 +bad_dict = {"1": 1, "2": 2, "3": 3, } + +# Correct: +good_dict = {"1": 1, "2": 2, "3": 3} + +# Wrong: C-819 +bad_tuple = (1, 2, 3,) + +# Correct +good_tuple = (1, 2, 3) diff --git a/test/resources/inspectors/python/case34_cohesion.py b/test/resources/inspectors/python/case34_cohesion.py new file mode 100644 index 00000000..8102675a --- /dev/null +++ b/test/resources/inspectors/python/case34_cohesion.py @@ -0,0 +1,28 @@ +from math import sqrt + + +class BadClass: + def __init__(self, x: int, y: int): + self.x = x + self.y = y + + @staticmethod + def length(x: int, y: int) -> float: + return sqrt(x ** 2 + y ** 2) + + @staticmethod + def dot(self_x: int, self_y: int, other_x: int, other_y: int) -> int: + return self_x * other_x + self_y * other_y + + +class GoodClass(object): + def __init__(self, x: int, y: int): + self.x = x + self.y = y + + @property + def length(self) -> float: + return sqrt(self.dot(self.x, self.y)) + + def dot(self, other_x: int, other_y: int) -> int: + return self.x * other_x + self.y * other_y diff --git a/test/resources/inspectors/python/case35_line_break.py b/test/resources/inspectors/python/case35_line_break.py new file mode 100644 index 00000000..ba718ea8 --- /dev/null +++ b/test/resources/inspectors/python/case35_line_break.py @@ -0,0 +1,54 @@ +# Wrong +from math import exp, \ + log + +# Correct +from math import ( + sin, + cos, +) + + +# Wrong +def do_something_wrong(x: float, y: float): + if sin(x) == cos(y) \ + and exp(x) == log(y): + print("Do not do that!") + + +print(do_something_wrong(1, 2)) + +# Wrong +wrong_string = "Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. " \ + + "Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, " \ + + "nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis, sem. " \ + + "Nulla consequat massa quis enim. Donec pede justo, fringilla vel, aliquet nec, " \ + + "vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, venenatis vitae, justo. " \ + + "Nullam dictum felis eu pede mollis pretium. Integer tincidunt. Cras dapibus. " \ + + "Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. Aenean leo ligula, " \ + + "porttitor eu, consequat vitae, eleifend ac, enim. Aliquam lorem ante, dapibus in, " \ + + "viverra quis, feugiat a," +print(wrong_string) + +# Correct +correct_string = ("Lorem ipsum dolor sit amet, consectetuer adipiscing elit. " + + "Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis " + + "parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, " + + "pretium quis, sem. Nulla consequat massa quis enim. Donec pede justo, fringilla vel, " + + "aliquet nec, vulputate eget, arcu. In enim justo, rhoncus ut, imperdiet a, " + + "venenatis vitae, justo. Nullam dictum felis eu pede mollis pretium. Integer tincidunt. " + + "Cras dapibus. Vivamus elementum semper nisi. Aenean vulputate eleifend tellus. " + + "Aenean leo ligula, porttitor eu, consequat vitae, eleifend ac, enim. " + + "Aliquam lorem ante, dapibus in, viverra quis, feugiat a," + ) +print(correct_string) + +other_correct_string = """ + Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec euismod vitae libero ut consequat. + Fusce quis ultrices sem, vitae viverra mi. Praesent fermentum quam ac volutpat condimentum. + Proin mauris orci, molestie vel fermentum vel, consectetur vel metus. Quisque vitae mollis magna. + In hac habitasse platea dictumst. Pellentesque sed diam eget dolor ultricies faucibus id sed quam. + Nam risus erat, varius ut risus a, tincidunt vulputate magna. Etiam lacinia a eros non faucibus. + In facilisis tempor nisl, sit amet feugiat lacus viverra quis. +""" +print(other_correct_string) diff --git a/test/resources/inspectors/python/case36_unpacking.py b/test/resources/inspectors/python/case36_unpacking.py new file mode 100644 index 00000000..74b4ec9c --- /dev/null +++ b/test/resources/inspectors/python/case36_unpacking.py @@ -0,0 +1,10 @@ +[a, b, c], [x, y, z] = (sorted(map(int, input().split())) for _ in 'lm') +if [a, b, c] == [x, y, z]: + a = "Boxes are equal" +elif a <= x and b <= y and c <= z: + a = "The first box is smaller than the second one" +elif a >= x and b >= y and c >= z: + a = "The first box is larger than the second one" +else: + a = "Boxes are incomparable" +print(a) diff --git a/test/resources/inspectors/python/case37_wildcard_import.py b/test/resources/inspectors/python/case37_wildcard_import.py new file mode 100644 index 00000000..ed12a4b1 --- /dev/null +++ b/test/resources/inspectors/python/case37_wildcard_import.py @@ -0,0 +1,33 @@ +from numpy import * + +n, m = [int(_) for _ in input().split()] +mat = zeros((n, m)) +s = 1 +for j in range(n): + for i in range(j, m - j): + if s > n * m: + break + mat[j][i] = s + s += 1 + + for i in range(j - n + 1, -j): + if s > n * m: + break + mat[i][-j - 1] = s + s += 1 + for i in range(-j - 2, -m + j - 1, -1): + if s > n * m: + break + mat[-j - 1][i] = s + s += 1 + + for i in range(-2 - j, -n + j, -1): + if s > n * m: + break + mat[i][j] = s + s += 1 + +for r in range(n): + for c in range(m): + print(str(int(mat[r][c])).ljust(2), end=' ') + print() diff --git a/test/resources/inspectors/python/case3_redefining_builtin.py b/test/resources/inspectors/python/case3_redefining_builtin.py index 81d7021b..ddcca3c4 100644 --- a/test/resources/inspectors/python/case3_redefining_builtin.py +++ b/test/resources/inspectors/python/case3_redefining_builtin.py @@ -1,4 +1,7 @@ a = int(input()) b = int(input()) -list = list(filter(lambda x: x % 3 == 0, range(a, b + 1))) + +range = range(a, b + 1) + +list = list(filter(lambda x: x % 3 == 0, range)) print(sum(list) / len(list)) diff --git a/test/resources/inspectors/python/case4_naming.py b/test/resources/inspectors/python/case4_naming.py index 7d4664e8..522ae0eb 100644 --- a/test/resources/inspectors/python/case4_naming.py +++ b/test/resources/inspectors/python/case4_naming.py @@ -13,7 +13,7 @@ def myFun(self): print('hello 1') def my_fun(self, QQ): - print('hello 2' + QQ) + print('hello 2 {}'.format(QQ)) @classmethod def test_fun(first): diff --git a/test/resources/inspectors/python/case7_empty_lines.py b/test/resources/inspectors/python/case7_empty_lines.py index 81b73b98..25760099 100644 --- a/test/resources/inspectors/python/case7_empty_lines.py +++ b/test/resources/inspectors/python/case7_empty_lines.py @@ -17,6 +17,7 @@ def minus_age(self, value): self.age -= value class AnotherClass: - pass + def do_something(self): + print(1) -print(10 + 20) +print(10) diff --git a/test/resources/inspectors/python_ast/__init__.py b/test/resources/inspectors/python_ast/__init__.py deleted file mode 100644 index e69de29b..00000000 diff --git a/whitelist.txt b/whitelist.txt index 8f29cf65..b1834c32 100644 --- a/whitelist.txt +++ b/whitelist.txt @@ -54,6 +54,35 @@ unlink utils param params +changelog +multiline +sqrt +WPS +OOP +mccabe +mcs +dicts +misrefactored +src +textwrap +dedent +maintainabilities +parsers +fs +KTS +nl +splitext +dirname +hyperstyle +XLSX +Eval +eval +openpyxl +dataframe +writelines +rmdir +df +unique # Springlint issues cbo dit @@ -61,3 +90,9 @@ lcom noc nom wmc +util +Namespace +case18 +case34 +removeprefix +toplevel