Compare commits

...

68 Commits

Author SHA1 Message Date
WebFreak001 dc907e4a24 upgrade libdparse 2025-03-23 13:56:16 +01:00
Jan Jurzitza 3a87c65bac
Update actions/upload-artifact 2025-03-01 01:01:41 +00:00
Hiroki Noda 796d212b05 Fix: add build type for macos-13 runner with dmd 2024-05-06 11:31:18 +02:00
Hiroki Noda a8c4a588b2 CI: specify macos-13 for DMD 2024-05-06 11:31:18 +02:00
Hiroki Noda cc1a2c0178 CI: update actions/checkout to v4 2024-05-06 11:31:18 +02:00
Hiroki Noda ff0a9bc2ee CI: restrict dmd to macOS latest 2024-05-06 11:31:18 +02:00
Hiroki Noda 565087aa76 [ci skip]: use indent style for yaml 2024-05-06 10:42:51 +02:00
Hiroki Noda fe8f7bd8bc chore: remove travis related things 2024-05-06 10:11:11 +02:00
Hiroki Noda 22c9f980ae Allow skipping checks for dscanner.suspicious.unmodified with nolint 2024-05-06 10:10:54 +02:00
Hiroki Noda 17f3286fef Clearify key names 2024-05-06 10:08:53 +02:00
ryuukk 433d1eb73e Print to stdout 2024-02-08 03:46:26 +01:00
SixthDot 9076f7bab3
docs(dscanner/utils): Update obsolete url in comment ()
Co-authored-by: Petar Kirov <petar.p.kirov@gmail.com>
2024-01-01 11:08:09 +02:00
Jeremy Baxter 01e90ec4d8 Fix build on BSD
Removed the line `SHELL:=/usr/bin/env bash'. Most BSDs don't ship bash in the
base system by default and the build doesn't need it anyway.

Also added some more version statements to define useXDG for the other BSDs.
2023-12-26 13:10:01 +01:00
WebFreak001 8612841365 fix compilation on old compilers 2023-10-25 08:49:37 +02:00
WebFreak001 42033dcc55 add BaseAnalyzerArguments to keep ctor changes sane
also immediately makes tokens a part of it

This struct can for example precompute token indices for line endings
2023-10-25 08:49:37 +02:00
ricardaxel 1e8f1ec9e6
Allow skipping checks with @("nolint(...)") and @nolint("...") ()
Co-authored-by: Axel Ricard <contact@axelricard.fr>
Co-authored-by: WebFreak001 <gh@webfreak.org>
2023-10-13 02:45:59 +02:00
Axel Ricard 69d824f4f7 introduce variable expandedArgs 2023-10-11 00:34:00 +02:00
Axel Ricard 3bf3f25f9a add --exclude cli option
This excludes given files or directory from linting
2023-10-11 00:34:00 +02:00
Axel Ricard 87f85c7db7 add some utils functions for path manipulation 2023-10-11 00:34:00 +02:00
Prajwal S N 159e9c9eec feat(highlight): support multiple themes
Signed-off-by: Prajwal S N <prajwalnadig21@gmail.com>
2023-09-24 19:36:21 +02:00
Robert Schadek b43c8f45cf Always Check Curly
Check that if|else|for|foreach|while|do|try|catch
are always followed by a BlockStatement aka. { }

closer

can not get the test to work

try to get the AutoFix in place

maybe a fix

nicer messages

some formatting

more tinkering

still nothing

autofix work now

AutoFix name

message to message_postfix
2023-09-24 19:35:46 +02:00
WebFreak001 fc1699bb97 simplify it.sh 2023-09-24 15:31:50 +02:00
WebFreak001 6491d792f5 support `@arguments.rst` for args through file 2023-09-24 15:31:50 +02:00
WebFreak001 a958f9ac7b fix unused variable check for unitthreaded checks
such as `a.should == b`
2023-07-17 14:41:07 +02:00
WebFreak001 c8262f4220 fix auto_function autofix for `auto ref fn()` 2023-07-17 11:32:16 +02:00
WebFreak001 f22b2e587c Disable auto_function_check by default
Since it may be used to auto-infer function attributes
2023-07-17 11:32:16 +02:00
WebFreak001 5d67707744 more sane parentheses fix for delegates
not sure what I was thinking with the initial version
2023-07-13 16:42:36 +02:00
WebFreak001 7601fe65f9 fix diagnostic location for `@UDA auto f() {}` 2023-07-10 22:05:26 +02:00
WebFreak001 c1e051bfba fix infinite allocating in context formatter 2023-07-10 13:57:27 +02:00
WebFreak001 48db254fb0 fix if scopes and shortened function bodies 2023-07-10 00:52:04 +02:00
WebFreak001 d275361153 fix case/default scopes, fix 2023-07-10 00:52:04 +02:00
Jan Jurzitza fed654441f
fix autofix CLI, add integration test for it () 2023-07-09 13:09:21 +02:00
Jan Jurzitza 4c759b072c
include resolved autofixes in `--report` output () 2023-07-09 09:44:02 +02:00
WebFreak001 cae7d595b8 checkout IT files with LF line endings 2023-07-09 00:45:42 +02:00
WebFreak001 5d3296cc0b it: only rebuild dscanner outside CI
fix windows redirects
2023-07-09 00:45:42 +02:00
WebFreak001 d7e15903dd run integration tests in CI 2023-07-09 00:45:42 +02:00
WebFreak001 0590376045 add IDE integration tests 2023-07-08 23:09:33 +02:00
WebFreak001 3345a1953a improve public auto fix API 2023-07-08 23:09:33 +02:00
WebFreak001 43caad72a8 add CLI API to get autofixes at location 2023-07-08 23:09:33 +02:00
WebFreak001 53c9536332 add startIndex, endIndex support to format string 2023-07-08 23:09:33 +02:00
WebFreak001 4194e6af0c add `dscanner fix` command 2023-07-08 23:09:33 +02:00
WebFreak001 48cec8a6f4 implement indentation aware autofixes 2023-07-08 23:09:33 +02:00
WebFreak001 93aae57469 add autofix testing API 2023-07-08 23:09:33 +02:00
WebFreak001 f12319d5a8 add autofix whitespace collapsing API 2023-07-08 23:09:33 +02:00
WebFreak001 513b7dafc3 add auto-fix API 2023-07-08 23:09:33 +02:00
Christian Köstlin 35d2cf4177 feature: Provide predefined error format compatible with dmds output
an output parser that works with dmd / ldc just works (tm) witha
dscanner as well
2023-07-07 00:15:04 +02:00
WebFreak001 1201a68f66 Only call Win32 API to enable colored output once 2023-07-05 22:41:42 +02:00
WebFreak001 b0bb905a40 upgrade libdparse to 0.23.2 fix mixin-type VarDecl 2023-07-05 21:37:52 +02:00
WebFreak001 a676bb13fb fix selective imports 2023-07-02 13:33:11 +02:00
WebFreak001 78f2b5a420 add colored output option
also adds a simpler way to invoke D-Scanner for users that uses this new
UI by default: `dscanner lint FILES...`
2023-07-02 13:33:11 +02:00
WebFreak001 3b8110fdfa phobos still has its own dscanner.ini 2023-06-30 13:59:33 +02:00
WebFreak001 eceb2743a8 rename .dscanner.ini to dscanner.ini
There doesn't really seem to be any reason to keep it a non-standard path.

Fix 
2023-06-30 13:59:33 +02:00
WebFreak001 146fec75d8 add index and endIndex to JSON formats 2023-06-29 17:43:30 +02:00
WebFreak001 b115a6333a also add byte indices to diagnostic ranges
For tools wanting to read from the source file this makes it much easier
to look up the code.
2023-06-29 17:43:30 +02:00
WebFreak001 bad253bad5 remove unused variables 2023-06-29 14:57:32 +02:00
WebFreak001 e3604d6ce6 only report linter warnings once 2023-06-29 14:57:32 +02:00
WebFreak001 83eb9c5c2e add built-in report formats incl. GitHub Actions
use on ourself and enable unused variables test to do first test in CI
2023-06-29 14:57:32 +02:00
WebFreak001 5c2035ff76 add end line/column to warnings 2023-06-29 13:19:36 +02:00
WebFreak001 5a53c538d0 make mismatched args not warn on named arguments
makes expressions be assumed as valid argument names
2023-05-23 12:15:52 +02:00
WebFreak001 4b2124e82d upgrade libdparse to 0.23.0 2023-05-23 12:15:52 +02:00
WebFreak001 93f338a5e7 fix logger warnings, for real this time
We should probably add a linter case for this
2023-05-19 23:12:16 +02:00
WebFreak001 f37faf85dd fix BodyOnDisabledFuncsCheck edge cases 2023-05-09 04:10:43 +02:00
Mai-Lapyst e2cc6e1ad2 Adds an check for `@disable`d functions that have an body; closes 2023-05-09 04:10:43 +02:00
Jan Jurzitza 5f1cf31ee0
hide dsymbol warnings, fix () 2023-05-09 03:34:08 +02:00
brianush1 d0c670a415 fix static warnings and add tests for function_attribute_check 2023-05-09 02:19:42 +02:00
brianush1 ba4617efac add unittest for FunctionAttributeCheck 2023-05-09 02:19:42 +02:00
brianush1 14ba4af4bd fix 2023-05-09 02:19:42 +02:00
Su 9b171c46d2
don't use deprecated properties () 2023-02-28 00:48:14 +01:00
88 changed files with 5398 additions and 1333 deletions

View File

@ -19,3 +19,7 @@ dfmt_space_after_keywords = true
dfmt_selective_import_space = true
dfmt_compact_labeled_statements = true
dfmt_template_constraint_style = conditional_newline_indent
[*.yml]
indent_style = space
indent_size = 2

1
.gitattributes vendored Normal file
View File

@ -0,0 +1 @@
tests/it/autofix_ide/source_autofix.d text eol=lf

View File

@ -57,18 +57,32 @@ jobs:
dmd: gdc-12
host: macos-latest
# Restrict DMD to macOS latest
- compiler:
dmd: dmd
host: macos-latest
# Omit dub builds for GDC because dub rejects the old fronted revision
- compiler:
dmd: gdc-12
build:
type: dub
include:
- { do_report: 1, build: { type: dub, version: 'current' }, host: 'ubuntu-22.04', compiler: { version: dmd-latest, dmd: dmd } }
- compiler:
dmd: dmd
host: macos-13
build:
type: 'dub'
version: 'current'
runs-on: ${{ matrix.host }}
steps:
# Clone repo + submodules
- name: Checkout repo
uses: actions/checkout@v2
uses: actions/checkout@v4
with:
submodules: 'recursive'
fetch-depth: 0
@ -121,7 +135,7 @@ jobs:
dub build
dub test
- uses: actions/upload-artifact@v2
- uses: actions/upload-artifact@v4
with:
name: bin-${{matrix.build.type}}-${{matrix.build.version}}-${{ matrix.compiler.dmd }}-${{ matrix.host }}
path: bin
@ -129,17 +143,29 @@ jobs:
# Lint source code using the previously built binary
- name: Run linter
shell: bash
env:
REPORT_GITHUB: ${{matrix.do_report}}
run: |
if [ "$RUNNER_OS" == "Windows" ]; then
EXE=".exe"
else
EXE=""
fi
"./bin/dscanner$EXE" --config .dscanner.ini --styleCheck src
if [ "$REPORT_GITHUB" == "1" ]; then
FORMAT="github"
else
FORMAT=""
fi
"./bin/dscanner$EXE" --styleCheck -f "$FORMAT" src
- name: Integration Tests
run: ./it.sh
working-directory: tests
shell: bash
# Parse phobos to check for failures / crashes / ...
- name: Checkout Phobos
uses: actions/checkout@v2
uses: actions/checkout@v4
with:
repository: dlang/phobos
path: phobos

View File

@ -1,24 +0,0 @@
#!/bin/bash
set -e
if [[ $BUILD == dub ]]; then
if [[ -n $LIBDPARSE_VERSION ]]; then
rdmd ./d-test-utils/test_with_package.d $LIBDPARSE_VERSION libdparse -- dub test
elif [[ -n $DSYMBOL_VERSION ]]; then
rdmd ./d-test-utils/test_with_package.d $DSYMBOL_VERSION dsymbol -- dub test
else
echo 'Cannot run test without LIBDPARSE_VERSION nor DSYMBOL_VERSION environment variable'
exit 1
fi
elif [[ $DC == ldc2 ]]; then
git submodule update --init --recursive
make test DC=ldmd2
else
git submodule update --init --recursive
make test
make lint
git clone https://www.github.com/dlang/phobos.git --depth=1
# just check that it doesn't crash
cd phobos/std && ../../bin/dscanner -S --config=../.dscanner.ini || true
fi

View File

@ -1,110 +0,0 @@
dist: xenial
sudo: false
language: d
d:
- dmd
- ldc-beta
- ldc
os:
- linux
- osx
env:
- BUILD=
- BUILD=dub LIBDPARSE_VERSION=min
- BUILD=dub LIBDPARSE_VERSION=max
- BUILD=dub DSYMBOL_VERSION=min
- BUILD=dub DSYMBOL_VERSION=max
branches:
only:
- master
- /^v\d+\.\d+\.\d+([+-]\S*)*$/
script: "./.travis.sh"
jobs:
include:
- stage: GitHub Release
#if: tag IS present
d: ldc-1.13.0
os: linux
script: echo "Deploying to GitHub releases ..." && ./release.sh
deploy:
provider: releases
api_key: $GH_REPO_TOKEN
file_glob: true
file: bin/dscanner-*.tar.gz
skip_cleanup: true
on:
repo: dlang-community/D-Scanner
tags: true
- stage: GitHub Release
#if: tag IS present
d: ldc-1.13.0
os: osx
script: echo "Deploying to GitHub releases ..." && ./release.sh
deploy:
provider: releases
api_key: $GH_REPO_TOKEN
file_glob: true
file: bin/dscanner-*.tar.gz
skip_cleanup: true
on:
repo: dlang-community/D-Scanner
tags: true
- stage: GitHub Release
#if: tag IS present
d: dmd
os: linux
language: generic
script: echo "Deploying to GitHub releases ..." && ./release-windows.sh
addons:
apt:
packages:
- p7zip-full
deploy:
provider: releases
api_key: $GH_REPO_TOKEN
file_glob: true
file: bin/dscanner-*.zip
skip_cleanup: true
on:
repo: dlang-community/D-Scanner
tags: true
- stage: GitHub Release
#if: tag IS present
d: dmd
os: linux
language: generic
script: echo "Deploying to GitHub releases ..." && ARCH=64 ./release-windows.sh
addons:
apt:
packages:
- p7zip-full
deploy:
provider: releases
api_key: $GH_REPO_TOKEN
file_glob: true
file: bin/dscanner-*.zip
skip_cleanup: true
on:
repo: dlang-community/D-Scanner
tags: true
- stage: dockerhub-stable
if: tag IS present
d: ldc
os: linux
script:
- echo "Deploying to DockerHub..." && ./release.sh
- LATEST_TAG="$(git describe --abbrev=0 --tags)"
- docker build -t "dlangcommunity/dscanner:${LATEST_TAG} ."
- if [[ "$TRAVIS_BRANCH" == "master" && "$TRAVIS_PULL_REQUEST" == "false" ]] ; then docker login -u="$DOCKER_USERNAME" -p="$DOCKER_PASSWORD" ; fi
- if [[ "$TRAVIS_BRANCH" == "master" && "$TRAVIS_PULL_REQUEST" == "false" ]] ; then docker push "dlangcommunity/dscanner:${LATEST_TAG}" ; fi
- stage: dockerhub-latest
d: ldc
os: linux
script:
- echo "Deploying to DockerHub..." && ./release.sh
- docker build -t dlangcommunity/dscanner:latest .
- if [[ "$TRAVIS_BRANCH" == "master" && "$TRAVIS_PULL_REQUEST" == "false" ]] ; then docker login -u="$DOCKER_USERNAME" -p="$DOCKER_PASSWORD" ; fi
- if [[ "$TRAVIS_BRANCH" == "master" && "$TRAVIS_PULL_REQUEST" == "false" ]] ; then docker push dlangcommunity/dscanner:latest ; fi
stages:
- name: test
if: type = pull_request or (type = push and branch = master)

2
DCD

@ -1 +1 @@
Subproject commit 4946d49abdc35810254151923bab30fb3cc2c004
Subproject commit 1c60c5480f70db568279e4637a5033953c777406

129
README.md
View File

@ -47,6 +47,109 @@ void main(string[] args)
}
```
## Linting
Use
```sh
dscanner lint source/
```
to view a human readable list of issues.
Diagnostic types can be enabled/disabled using a configuration file, check out
the `--config` argument / `dscanner.ini` file for more info. Tip: some IDEs that
integrate D-Scanner may have helpers to configure the diagnostics or help
generate the dscanner.ini file.
<!--
IDE list for overview:
code-d has an "insert default dscanner.ini content" command + proprietary
disabling per-line (we really need to bring that into standard D-Scanner)
-->
## Auto-Fixing issues
Use
```sh
dscanner fix source/
```
to interactively fix all fixable issues within the source directory. Call with
`--applySingle` to automatically apply fixes that don't have multiple automatic
solutions.
## Tooling integration
Many D editors already ship with D-Scanner.
For a CLI / tool parsable output use either
```sh
dscanner -S source/
# or
dscanner --report source/
```
The `--report` switch includes all information, plus cheap to compute autofixes
that are already resolved ahead of time, as well as the names for the autofixes
that need to be resolved using the `--resolveMessage` switch like described
below.
You can also specify custom formats using `-f` / `--errorFormat`, where there
are also built-in formats for GitHub Actions:
```sh
# for GitHub actions: (automatically adds annotations to files in PRs)
dscanner -S -f github source/
# custom format:
dscanner -S -f '{filepath}({line}:{column})[{type}]: {message}' source/
```
To resolve automatic issue fixes for a given location use
```sh
# collecting automatic issue fixes
# --resolveMessage <line>:<column> <filename>
dscanner --resolveMessage 11:3 file.d
# --resolveMessage b<byteIndex> <filename>
dscanner --resolveMessage b512 file.d
# <filename> may be omitted to read from stdin
```
outputs JSON:
```json
// list of available auto-fixes at the given location
[
{
"name": "Make function const",
// byte range `[start, end)` what code to replace
// this is sorted by range[0]
"replacements": [
// replace: range[0] < range[1], newText != ""
{"range": [10, 14], "newText": "const "},
// insert: range[0] == range[1], newText != ""
{"range": [20, 20], "newText": "auto"},
// remove: range[0] < range[1], newText == ""
{"range": [30, 40], "newText": ""},
]
}
]
```
Algorithm to apply replacements:
```d
foreach_reverse (r; replacements)
codeBytes = codeBytes[0 .. r.range[0]] ~ r.newText ~ codeBytes[r.range[1] .. $];
```
Replacements are non-overlapping, sorted by `range[0]` in ascending order. When
combining multiple different replacements, you first need to sort them by
`range[0]` to apply using the algorithm above.
## Other features
### Token Count
The "--tokenCount" or "-t" option prints the number of tokens in the given file
@ -99,7 +202,7 @@ To avoid these cases, it's possible to pass the "--skipTests" option.
#### Configuration
By default all checks are enabled. Individual checks can be enabled or disabled
by using a configuration file. Such a file can be placed, for example, is the root directory of your project.
Running ```dscanner --defaultConfig``` will generate a default configuration file and print the file's location.
Running `dscanner --defaultConfig` will generate a default configuration file and print the file's location.
You can also specify the path to a configuration file by using the "--config" option if
you want to override the default or the local settings.
@ -203,8 +306,15 @@ and case tokens in the file.
### Syntax Highlighting
The "--highlight" option prints the given source file as syntax-highlighted HTML
to the standard output. The CSS styling is currently hard-coded to use the
[Solarized](http://ethanschoonover.com/solarized) color scheme.
to the standard output. The CSS styling uses the [Solarized](http://ethanschoonover.com/solarized)
color scheme by default, but can be customised using the "--theme" option.
The following themes are available:
- `solarized`
- `solarized-dark`
- `gruvbox`
- `gruvbox-dark`
No example. It would take up too much space
@ -247,10 +357,13 @@ outline of the file's declarations to stdout.
### Configuration
By default Dscanner uses the configuration file given in `$HOME/.config/dscanner/dscanner.ini`.
Run `--defaultConfig` to regenerate it.
The `--config` option allows one to use a custom configuration file.
If a `dscanner.ini` file is locate in the working directory or any of it's parents, it overrides any other configuration files.
If a `dscanner.ini` file is locate in the working directory or any of it's
parents, it overrides any other configuration files.
As final location, D-Scanner uses the configuration file given in
`$HOME/.config/dscanner/dscanner.ini`. Run `--defaultConfig` to regenerate it.
The `--config` option allows one to use a custom configuration file path.
### AST Dump
The "--ast" or "--xml" options will dump the complete abstract syntax tree of
@ -347,7 +460,7 @@ using its formatting switch.
Selecting modules for a specific check
--------------------------------------
It is possible to create a new section `analysis.config.ModuleFilters` in the `.dscanner.ini`.
It is possible to create a new section `analysis.config.ModuleFilters` in the `dscanner.ini`.
In this optional section a comma-separated list of inclusion and exclusion selectors can
be specified for every check on which selective filtering should be applied.
These given selectors match on the module name and partial matches (`std.` or `.foo.`) are possible.

View File

@ -18,8 +18,8 @@ if %githashsize% == 0 (
move /y bin\githash_.txt bin\githash.txt
)
set DFLAGS=-O -release -version=StdLoggerDisableWarning -Jbin %MFLAGS%
set TESTFLAGS=-g -w -version=StdLoggerDisableWarning -Jbin
set DFLAGS=-O -release -Jbin %MFLAGS%
set TESTFLAGS=-g -w -Jbin
set CORE=
set LIBDPARSE=
set STD=

View File

@ -23,7 +23,7 @@ if_else_same_check="enabled"
; Checks for some problems with constructors
constructor_check="enabled"
; Checks for unused variables and function parameters
unused_variable_check="disabled"
unused_variable_check="enabled"
; Checks for unused labels
unused_label_check="enabled"
; Checks for duplicate attributes
@ -97,3 +97,5 @@ unused_result="enabled"
cyclomatic_complexity="disabled"
; Maximum cyclomatic complexity after which to issue warnings
max_cyclomatic_complexity="50"
; Check for function bodies on disabled functions
body_on_disabled_func_check="enabled"

View File

@ -8,12 +8,11 @@
"license" : "BSL-1.0",
"targetType" : "autodetect",
"versions" : [
"built_with_dub",
"StdLoggerDisableWarning"
"built_with_dub"
],
"dependencies": {
"libdparse": ">=0.20.0 <0.23.0",
"dcd:dsymbol": ">=0.14.0 <0.16.0",
"libdparse": ">=0.23.1 <0.26.0",
"dcd:dsymbol": ">=0.16.0-beta.2 <0.17.0",
"inifiled": "~>1.3.1",
"emsi_containers": "~>0.9.0",
"libddoc": "~>0.8.0"

View File

@ -1,12 +1,12 @@
{
"fileVersion": 1,
"versions": {
"dcd": "0.15.2",
"dcd": "0.16.0-beta.2",
"dsymbol": "0.13.0",
"emsi_containers": "0.9.0",
"inifiled": "1.3.3",
"libddoc": "0.8.0",
"libdparse": "0.22.0",
"libdparse": "0.25.0",
"stdx-allocator": "2.77.5"
}
}

@ -1 +1 @@
Subproject commit 98bf0f4166578717e0b78472ff5054d6f918e797
Subproject commit f8a6c28589aae180532fb460a1b22e92a0978292

View File

@ -44,11 +44,14 @@ INCLUDE_PATHS = \
-Ilibddoc/src \
-Ilibddoc/common/source
DMD_VERSIONS = -version=StdLoggerDisableWarning
# e.g. "-version=MyCustomVersion"
DMD_VERSIONS =
DMD_DEBUG_VERSIONS = -version=dparse_verbose
LDC_VERSIONS = -d-version=StdLoggerDisableWarning
# e.g. "-d-version=MyCustomVersion"
LDC_VERSIONS =
LDC_DEBUG_VERSIONS = -d-version=dparse_verbose
GDC_VERSIONS = -fversion=StdLoggerDisableWarning
# e.g. "-fversion=MyCustomVersion"
GDC_VERSIONS =
GDC_DEBUG_VERSIONS = -fversion=dparse_verbose
DC_FLAGS += -Jbin
@ -83,8 +86,6 @@ else ifneq (,$(findstring gdc, $(DC)))
WRITE_TO_TARGET_NAME = -o $@
endif
SHELL:=/usr/bin/env bash
GITHASH = bin/githash.txt
@ -136,7 +137,7 @@ ${UT_DSCANNER_BIN}: ${UT_DSCANNER_LIB} ${GITHASH} ${UT_OBJ_BY_DC} | ${DSCANNER_B
./${UT_DSCANNER_BIN}
lint: ${DSCANNER_BIN}
./${DSCANNER_BIN} --config .dscanner.ini --styleCheck src
./${DSCANNER_BIN} --styleCheck src
clean:
rm -rf dsc

View File

@ -18,9 +18,9 @@ final class AliasSyntaxCheck : BaseAnalyzer
mixin AnalyzerInfo!"alias_syntax_check";
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const AliasDeclaration ad)
@ -29,8 +29,7 @@ final class AliasSyntaxCheck : BaseAnalyzer
return;
assert(ad.declaratorIdentifierList.identifiers.length > 0,
"Identifier list length is zero, libdparse has a bug");
addErrorMessage(ad.declaratorIdentifierList.identifiers[0].line,
ad.declaratorIdentifierList.identifiers[0].column, KEY,
addErrorMessage(ad, KEY,
"Prefer the new \"'alias' identifier '=' type ';'\" syntax"
~ " to the old \"'alias' type identifier ';'\" syntax.");
}
@ -48,7 +47,8 @@ unittest
StaticAnalysisConfig sac = disabledConfig();
sac.alias_syntax_check = Check.enabled;
assertAnalyzerWarnings(q{
alias int abcde; // [warn]: Prefer the new "'alias' identifier '=' type ';'" syntax to the old "'alias' type identifier ';'" syntax.
alias int abcde; /+
^^^^^^^^^^^^^^^^ [warn]: Prefer the new "'alias' identifier '=' type ';'" syntax to the old "'alias' type identifier ';'" syntax.+/
alias abcde = int;
}c, sac);

View File

@ -30,9 +30,9 @@ final class AllManCheck : BaseAnalyzer
mixin AnalyzerInfo!"allman_braces_check";
///
this(string fileName, const(Token)[] tokens, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
foreach (i; 1 .. tokens.length - 1)
{
const curLine = tokens[i].line;
@ -47,7 +47,7 @@ final class AllManCheck : BaseAnalyzer
continue;
// ignore inline { } braces
if (curLine != tokens[i + 1].line)
addErrorMessage(tokens[i].line, tokens[i].column, KEY, MESSAGE);
addErrorMessage(tokens[i], KEY, MESSAGE);
}
if (tokens[i].type == tok!"}" && curLine == prevTokenLine)
{
@ -56,7 +56,7 @@ final class AllManCheck : BaseAnalyzer
continue;
// ignore inline { } braces
if (!tokens[0 .. i].retro.until!(t => t.line != curLine).canFind!(t => t.type == tok!"{"))
addErrorMessage(tokens[i].line, tokens[i].column, KEY, MESSAGE);
addErrorMessage(tokens[i], KEY, MESSAGE);
}
}
}
@ -79,7 +79,8 @@ unittest
assertAnalyzerWarnings(q{
void testAllman()
{
while (true) { // [warn]: %s
while (true) { /+
^ [warn]: %s +/
auto f = 1;
}

View File

@ -0,0 +1,227 @@
// Distributed under the Boost Software License, Version 1.0.
// (See accompanying file LICENSE_1_0.txt or copy at
// http://www.boost.org/LICENSE_1_0.txt)
module dscanner.analysis.always_curly;
import dparse.lexer;
import dparse.ast;
import dscanner.analysis.base;
import dsymbol.scope_ : Scope;
import std.array : back, front;
import std.algorithm;
import std.range;
import std.stdio;
final class AlwaysCurlyCheck : BaseAnalyzer
{
mixin AnalyzerInfo!"always_curly_check";
alias visit = BaseAnalyzer.visit;
///
this(BaseAnalyzerArguments args)
{
super(args);
}
void test(L, B)(L loc, B s, string stmtKind)
{
if (!is(s == BlockStatement))
{
if (!s.tokens.empty)
{
AutoFix af = AutoFix.insertionBefore(s.tokens.front, " { ")
.concat(AutoFix.insertionAfter(s.tokens.back, " } "));
af.name = "Wrap in braces";
addErrorMessage(loc, KEY, stmtKind ~ MESSAGE_POSTFIX, [af]);
}
else
{
addErrorMessage(loc, KEY, stmtKind ~ MESSAGE_POSTFIX);
}
}
}
override void visit(const(IfStatement) stmt)
{
auto s = stmt.thenStatement.statement;
this.test(stmt.thenStatement, s, "if");
if (stmt.elseStatement !is null)
{
auto e = stmt.elseStatement.statement;
this.test(stmt.elseStatement, e, "else");
}
}
override void visit(const(ForStatement) stmt)
{
auto s = stmt.declarationOrStatement;
if (s.statement !is null)
{
this.test(s, s, "for");
}
}
override void visit(const(ForeachStatement) stmt)
{
auto s = stmt.declarationOrStatement;
if (s.statement !is null)
{
this.test(s, s, "foreach");
}
}
override void visit(const(TryStatement) stmt)
{
auto s = stmt.declarationOrStatement;
if (s.statement !is null)
{
this.test(s, s, "try");
}
if (stmt.catches !is null)
{
foreach (const(Catch) ct; stmt.catches.catches)
{
this.test(ct, ct.declarationOrStatement, "catch");
}
if (stmt.catches.lastCatch !is null)
{
auto sncnd = stmt.catches.lastCatch.statementNoCaseNoDefault;
if (sncnd !is null)
{
this.test(stmt.catches.lastCatch, sncnd, "finally");
}
}
}
}
override void visit(const(WhileStatement) stmt)
{
auto s = stmt.declarationOrStatement;
if (s.statement !is null)
{
this.test(s, s, "while");
}
}
override void visit(const(DoStatement) stmt)
{
auto s = stmt.statementNoCaseNoDefault;
if (s !is null)
{
this.test(s, s, "do");
}
}
enum string KEY = "dscanner.style.always_curly";
enum string MESSAGE_POSTFIX = " must be follow by a BlockStatement aka. { }";
}
unittest
{
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig();
sac.always_curly_check = Check.enabled;
assertAnalyzerWarnings(q{
void testIf()
{
if(true) return; // [warn]: if must be follow by a BlockStatement aka. { }
}
}, sac);
assertAnalyzerWarnings(q{
void testIf()
{
if(true) return; /+
^^^^^^^ [warn]: if must be follow by a BlockStatement aka. { } +/
}
}, sac);
assertAnalyzerWarnings(q{
void testIf()
{
for(int i = 0; i < 10; ++i) return; // [warn]: for must be follow by a BlockStatement aka. { }
}
}, sac);
assertAnalyzerWarnings(q{
void testIf()
{
foreach(it; 0 .. 10) return; // [warn]: foreach must be follow by a BlockStatement aka. { }
}
}, sac);
assertAnalyzerWarnings(q{
void testIf()
{
while(true) return; // [warn]: while must be follow by a BlockStatement aka. { }
}
}, sac);
assertAnalyzerWarnings(q{
void testIf()
{
do return; while(true); return; // [warn]: do must be follow by a BlockStatement aka. { }
}
}, sac);
}
unittest {
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig();
sac.always_curly_check = Check.enabled;
assertAutoFix(q{
void test() {
if(true) return; // fix:0
}
}c, q{
void test() {
if(true) { return; } // fix:0
}
}c, sac);
assertAutoFix(q{
void test() {
foreach(_; 0 .. 10 ) return; // fix:0
}
}c, q{
void test() {
foreach(_; 0 .. 10 ) { return; } // fix:0
}
}c, sac);
assertAutoFix(q{
void test() {
for(int i = 0; i < 10; ++i) return; // fix:0
}
}c, q{
void test() {
for(int i = 0; i < 10; ++i) { return; } // fix:0
}
}c, sac);
assertAutoFix(q{
void test() {
do return; while(true) // fix:0
}
}c, q{
void test() {
do { return; } while(true) // fix:0
}
}c, sac);
stderr.writeln("Unittest for AlwaysCurly passed.");
}

View File

@ -22,9 +22,9 @@ final class AsmStyleCheck : BaseAnalyzer
mixin AnalyzerInfo!"asm_style_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const AsmBrExp brExp)
@ -32,11 +32,13 @@ final class AsmStyleCheck : BaseAnalyzer
if (brExp.asmBrExp !is null && brExp.asmBrExp.asmUnaExp !is null
&& brExp.asmBrExp.asmUnaExp.asmPrimaryExp !is null)
{
addErrorMessage(brExp.line, brExp.column, "dscanner.confusing.brexp",
addErrorMessage(brExp, KEY,
"This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.");
}
brExp.accept(this);
}
private enum string KEY = "dscanner.confusing.brexp";
}
unittest
@ -50,7 +52,8 @@ unittest
{
asm
{
mov a, someArray[1]; // [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify.
mov a, someArray[1]; /+
^^^^^^^^^^^^ [warn]: This is confusing because it looks like an array index. Rewrite a[1] as [a + 1] to clarify. +/
add near ptr [EAX], 3;
}
}

View File

@ -23,9 +23,9 @@ final class AssertWithoutMessageCheck : BaseAnalyzer
mixin AnalyzerInfo!"assert_without_msg";
///
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const AssertExpression expr)
@ -41,7 +41,7 @@ final class AssertWithoutMessageCheck : BaseAnalyzer
&& expr.assertArguments.message !is null;
if (!hasMessage)
addErrorMessage(expr.line, expr.column, KEY, MESSAGE);
addErrorMessage(expr, KEY, MESSAGE);
}
override void visit(const FunctionCallExpression expr)
@ -53,9 +53,9 @@ final class AssertWithoutMessageCheck : BaseAnalyzer
.unaryExpression.primaryExpression.identifierOrTemplateInstance)
{
auto ident = iot.identifier;
if (ident.text == "enforce" && expr.arguments !is null && expr.arguments.argumentList !is null &&
expr.arguments.argumentList.items.length < 2)
addErrorMessage(ident.line, ident.column, KEY, MESSAGE);
if (ident.text == "enforce" && expr.arguments !is null && expr.arguments.namedArgumentList !is null &&
expr.arguments.namedArgumentList.items.length < 2)
addErrorMessage(expr, KEY, MESSAGE);
}
}
@ -112,7 +112,8 @@ unittest
assertAnalyzerWarnings(q{
unittest {
assert(0, "foo bar");
assert(0); // [warn]: %s
assert(0); /+
^^^^^^^^^ [warn]: %s +/
}
}c.format(
AssertWithoutMessageCheck.MESSAGE,
@ -121,7 +122,8 @@ unittest
assertAnalyzerWarnings(q{
unittest {
static assert(0, "foo bar");
static assert(0); // [warn]: %s
static assert(0); /+
^^^^^^^^^ [warn]: %s +/
}
}c.format(
AssertWithoutMessageCheck.MESSAGE,
@ -133,7 +135,8 @@ unittest
enforce(0); // std.exception not imported yet - could be a user-defined symbol
import std.exception;
enforce(0, "foo bar");
enforce(0); // [warn]: %s
enforce(0); /+
^^^^^^^^^^ [warn]: %s +/
}
}c.format(
AssertWithoutMessageCheck.MESSAGE,

View File

@ -11,7 +11,7 @@ import dparse.ast;
import dparse.lexer;
import std.stdio;
import std.algorithm.searching : any;
import std.algorithm : map, filter;
/**
* Checks for auto functions without return statement.
@ -26,7 +26,8 @@ final class AutoFunctionChecker : BaseAnalyzer
private:
enum string KEY = "dscanner.suspicious.missing_return";
enum string MESSAGE = "Auto function without return statement, prefer an explicit void";
enum string MESSAGE = "Auto function without return statement, prefer replacing auto with void";
enum string MESSAGE_INSERT = "Auto function without return statement, prefer inserting void to be explicit";
bool[] _returns;
size_t _mixinDepth;
@ -39,9 +40,22 @@ public:
mixin AnalyzerInfo!"auto_function_check";
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
package static const(Token)[] findAutoReturnType(const(FunctionDeclaration) decl)
{
const(Token)[] lastAtAttribute;
foreach (storageClass; decl.storageClasses)
{
if (storageClass.token.type == tok!"auto")
return storageClass.tokens;
else if (storageClass.atAttribute)
lastAtAttribute = storageClass.atAttribute.tokens;
}
return lastAtAttribute;
}
override void visit(const(FunctionDeclaration) decl)
@ -50,13 +64,27 @@ public:
scope(exit) _returns.length -= 1;
_returns[$-1] = false;
const bool autoFun = decl.storageClasses
.any!(a => a.token.type == tok!"auto" || a.atAttribute !is null);
auto autoTokens = findAutoReturnType(decl);
bool isAtAttribute = autoTokens.length > 1;
decl.accept(this);
if (decl.functionBody.specifiedFunctionBody && autoFun && !_returns[$-1])
addErrorMessage(decl.name.line, decl.name.column, KEY, MESSAGE);
if (decl.functionBody.specifiedFunctionBody && autoTokens.length && !_returns[$-1])
{
if (isAtAttribute)
{
// highlight on the whitespace between attribute and function name
auto tok = autoTokens[$ - 1];
auto whitespace = tok.column + (tok.text.length ? tok.text.length : str(tok.type).length);
auto whitespaceIndex = tok.index + (tok.text.length ? tok.text.length : str(tok.type).length);
addErrorMessage([whitespaceIndex, whitespaceIndex + 1], tok.line, [whitespace, whitespace + 1], KEY, MESSAGE_INSERT,
[AutoFix.insertionAt(whitespaceIndex + 1, "void ")]);
}
else
addErrorMessage(autoTokens, KEY, MESSAGE,
[AutoFix.replacement(autoTokens[0], "", "Replace `auto` with `void`")
.concat(AutoFix.insertionAt(decl.name.index, "void "))]);
}
}
override void visit(const(ReturnStatement) rst)
@ -165,67 +193,82 @@ unittest
StaticAnalysisConfig sac = disabledConfig();
sac.auto_function_check = Check.enabled;
assertAnalyzerWarnings(q{
auto ref doStuff(){} // [warn]: %s
auto doStuff(){} // [warn]: %s
int doStuff(){auto doStuff(){}} // [warn]: %s
auto ref doStuff(){} /+
^^^^ [warn]: %s +/
auto doStuff(){} /+
^^^^ [warn]: %s +/
@Custom
auto doStuff(){} /+
^^^^ [warn]: %s +/
int doStuff(){auto doStuff(){}} /+
^^^^ [warn]: %s +/
auto doStuff(){return 0;}
int doStuff(){/*error but not the aim*/}
}c.format(
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){assert(true);} // [warn]: %s
auto doStuff(){assert(true);} /+
^^^^ [warn]: %s +/
auto doStuff(){assert(false);}
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){assert(1);} // [warn]: %s
auto doStuff(){assert(1);} /+
^^^^ [warn]: %s +/
auto doStuff(){assert(0);}
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){mixin("0+0");} // [warn]: %s
auto doStuff(){mixin("0+0");} /+
^^^^ [warn]: %s +/
auto doStuff(){mixin("return 0;");}
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){mixin("0+0");} // [warn]: %s
auto doStuff(){mixin("0+0");} /+
^^^^ [warn]: %s +/
auto doStuff(){mixin("static if (true)" ~ " return " ~ 0.stringof ~ ";");}
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){} // [warn]: %s
auto doStuff(){} /+
^^^^ [warn]: %s +/
extern(C) auto doStuff();
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
auto doStuff(){} // [warn]: %s
auto doStuff(){} /+
^^^^ [warn]: %s +/
@disable auto doStuff();
}c.format(
AutoFunctionChecker.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
@property doStuff(){} // [warn]: %s
@safe doStuff(){} // [warn]: %s
@property doStuff(){} /+
^ [warn]: %s +/
@safe doStuff(){} /+
^ [warn]: %s +/
@disable doStuff();
@safe void doStuff();
}c.format(
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE,
AutoFunctionChecker.MESSAGE_INSERT,
AutoFunctionChecker.MESSAGE_INSERT,
), sac);
assertAnalyzerWarnings(q{
@ -233,5 +276,22 @@ unittest
auto doStuff(){ mixin(_genSave);}
}, sac);
assertAutoFix(q{
auto ref doStuff(){} // fix
auto doStuff(){} // fix
@property doStuff(){} // fix
@safe doStuff(){} // fix
@Custom
auto doStuff(){} // fix
}c, q{
ref void doStuff(){} // fix
void doStuff(){} // fix
@property void doStuff(){} // fix
@safe void doStuff(){} // fix
@Custom
void doStuff(){} // fix
}c, sac);
stderr.writeln("Unittest for AutoFunctionChecker passed.");
}

View File

@ -17,9 +17,9 @@ final class AutoRefAssignmentCheck : BaseAnalyzer
mixin AnalyzerInfo!"auto_ref_assignment_check";
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const Module m)
@ -54,29 +54,29 @@ final class AutoRefAssignmentCheck : BaseAnalyzer
{
if (assign.operator == tok!"" || scopes.length == 0)
return;
interest++;
interest ~= assign;
assign.ternaryExpression.accept(this);
interest--;
interest.length--;
}
override void visit(const IdentifierOrTemplateInstance ioti)
{
import std.algorithm.searching : canFind;
if (ioti.identifier == tok!"" || interest <= 0)
if (ioti.identifier == tok!"" || !interest.length)
return;
if (scopes[$ - 1].canFind(ioti.identifier.text))
addErrorMessage(ioti.identifier.line, ioti.identifier.column, KEY, MESSAGE);
addErrorMessage(interest[$ - 1], KEY, MESSAGE);
}
override void visit(const IdentifierChain ic)
{
import std.algorithm.searching : canFind;
if (ic.identifiers.length == 0 || interest <= 0)
if (ic.identifiers.length == 0 || !interest.length)
return;
if (scopes[$ - 1].canFind(ic.identifiers[0].text))
addErrorMessage(ic.identifiers[0].line, ic.identifiers[0].column, KEY, MESSAGE);
addErrorMessage(interest[$ - 1], KEY, MESSAGE);
}
alias visit = BaseAnalyzer.visit;
@ -86,12 +86,7 @@ private:
enum string MESSAGE = "Assignment to auto-ref function parameter.";
enum string KEY = "dscanner.suspicious.auto_ref_assignment";
invariant
{
assert(interest >= 0);
}
int interest;
const(AssignExpression)[] interest;
void addSymbol(string symbolName)
{
@ -123,7 +118,8 @@ unittest
assertAnalyzerWarnings(q{
int doStuff(T)(auto ref int a)
{
a = 10; // [warn]: %s
a = 10; /+
^^^^^^ [warn]: %s +/
}
int doStuff(T)(ref int a)

View File

@ -1,28 +1,363 @@
module dscanner.analysis.base;
import std.container;
import std.string;
import dparse.ast;
import std.array;
import dparse.lexer : IdType, str, Token, tok;
import dscanner.analysis.nolint;
import dsymbol.scope_ : Scope;
import std.array;
import std.container;
import std.meta : AliasSeq;
import std.string;
import std.sumtype;
struct Message
///
struct AutoFix
{
/// Name of the file where the warning was triggered
string fileName;
/// Line number where the warning was triggered
size_t line;
/// Column number where the warning was triggered (in bytes)
size_t column;
/// Name of the warning
string key;
/// Warning message
string message;
/// Check name
string checkName;
///
struct CodeReplacement
{
/// Byte index `[start, end)` within the file what text to replace.
/// `start == end` if text is only getting inserted.
size_t[2] range;
/// The new text to put inside the range. (empty to delete text)
string newText;
}
/// Context that the analyzer resolve method can use to generate the
/// resolved `CodeReplacement` with.
struct ResolveContext
{
/// Arbitrary analyzer-defined parameters. May grow in the future with
/// more items.
ulong[3] params;
/// For dynamically sized data, may contain binary data.
string extraInfo;
}
/// Display name for the UI.
string name;
/// Either code replacements, sorted by range start, never overlapping, or a
/// context that can be passed to `BaseAnalyzer.resolveAutoFix` along with
/// the message key from the parent `Message` object.
///
/// `CodeReplacement[]` should be applied to the code in reverse, otherwise
/// an offset to the following start indices must be calculated and be kept
/// track of.
SumType!(CodeReplacement[], ResolveContext) replacements;
invariant
{
replacements.match!(
(const CodeReplacement[] replacement)
{
import std.algorithm : all, isSorted;
assert(replacement.all!"a.range[0] <= a.range[1]");
assert(replacement.isSorted!"a.range[0] < b.range[0]");
},
(_) {}
);
}
static AutoFix resolveLater(string name, ulong[3] params, string extraInfo = null)
{
AutoFix ret;
ret.name = name;
ret.replacements = ResolveContext(params, extraInfo);
return ret;
}
static AutoFix replacement(const Token token, string newText, string name = null)
{
if (!name.length)
{
auto text = token.text.length ? token.text : str(token.type);
if (newText.length)
name = "Replace `" ~ text ~ "` with `" ~ newText ~ "`";
else
name = "Remove `" ~ text ~ "`";
}
return replacement([token], newText, name);
}
static AutoFix replacement(const BaseNode node, string newText, string name)
{
return replacement(node.tokens, newText, name);
}
static AutoFix replacement(const Token[] tokens, string newText, string name)
in(tokens.length > 0, "must provide at least one token")
{
auto end = tokens[$ - 1].text.length ? tokens[$ - 1].text : str(tokens[$ - 1].type);
return replacement([tokens[0].index, tokens[$ - 1].index + end.length], newText, name);
}
static AutoFix replacement(size_t[2] range, string newText, string name)
{
AutoFix ret;
ret.name = name;
ret.replacements = [
AutoFix.CodeReplacement(range, newText)
];
return ret;
}
static AutoFix insertionBefore(const Token token, string content, string name = null)
{
return insertionAt(token.index, content, name);
}
static AutoFix insertionAfter(const Token token, string content, string name = null)
{
auto tokenText = token.text.length ? token.text : str(token.type);
return insertionAt(token.index + tokenText.length, content, name);
}
static AutoFix insertionAt(size_t index, string content, string name = null)
{
assert(content.length > 0, "generated auto fix inserting text without content");
AutoFix ret;
ret.name = name.length
? name
: content.strip.length
? "Insert `" ~ content.strip ~ "`"
: "Insert whitespace";
ret.replacements = [
AutoFix.CodeReplacement([index, index], content)
];
return ret;
}
static AutoFix indentLines(scope const(Token)[] tokens, const AutoFixFormatting formatting, string name = "Indent code")
{
CodeReplacement[] inserts;
size_t line = -1;
foreach (token; tokens)
{
if (line != token.line)
{
line = token.line;
inserts ~= CodeReplacement([token.index, token.index], formatting.indentation);
}
}
AutoFix ret;
ret.name = name;
ret.replacements = inserts;
return ret;
}
AutoFix concat(AutoFix other) const
{
import std.algorithm : sort;
static immutable string errorMsg = "Cannot concatenate code replacement with late-resolve";
AutoFix ret;
ret.name = name;
CodeReplacement[] concatenated = expectReplacements(errorMsg).dup
~ other.expectReplacements(errorMsg);
concatenated.sort!"a.range[0] < b.range[0]";
ret.replacements = concatenated;
return ret;
}
CodeReplacement[] expectReplacements(
string errorMsg = "Expected available code replacements, not something to resolve later"
) @safe pure nothrow @nogc
{
return replacements.match!(
(replacement)
{
if (false) return CodeReplacement[].init;
static if (is(immutable typeof(replacement) == immutable CodeReplacement[]))
return replacement;
else
assert(false, errorMsg);
}
);
}
const(CodeReplacement[]) expectReplacements(
string errorMsg = "Expected available code replacements, not something to resolve later"
) const @safe pure nothrow @nogc
{
return replacements.match!(
(const replacement)
{
if (false) return CodeReplacement[].init;
static if (is(immutable typeof(replacement) == immutable CodeReplacement[]))
return replacement;
else
assert(false, errorMsg);
}
);
}
}
enum comparitor = q{ a.line < b.line || (a.line == b.line && a.column < b.column) };
/// Formatting style for autofix generation (only available for resolve autofix)
struct AutoFixFormatting
{
enum AutoFixFormatting invalid = AutoFixFormatting(BraceStyle.invalid, null, 0, null);
enum BraceStyle
{
/// invalid, shouldn't appear in usable configs
invalid,
/// $(LINK https://en.wikipedia.org/wiki/Indent_style#Allman_style)
allman,
/// $(LINK https://en.wikipedia.org/wiki/Indent_style#Variant:_1TBS)
otbs,
/// $(LINK https://en.wikipedia.org/wiki/Indent_style#Variant:_Stroustrup)
stroustrup,
/// $(LINK https://en.wikipedia.org/wiki/Indentation_style#K&R_style)
knr,
}
/// Brace style config
BraceStyle braceStyle = BraceStyle.allman;
/// String to insert on indentations
string indentation = "\t";
/// For calculating indentation size
uint indentationWidth = 4;
/// String to insert on line endings
string eol = "\n";
invariant
{
import std.algorithm : all;
assert(!indentation.length
|| indentation == "\t"
|| indentation.all!(c => c == ' '));
}
string getWhitespaceBeforeOpeningBrace(string lastLineIndent, bool isFuncDecl) pure nothrow @safe const
{
final switch (braceStyle)
{
case BraceStyle.invalid:
assert(false, "invalid formatter config");
case BraceStyle.knr:
if (isFuncDecl)
goto case BraceStyle.allman;
else
goto case BraceStyle.otbs;
case BraceStyle.otbs:
case BraceStyle.stroustrup:
return " ";
case BraceStyle.allman:
return eol ~ lastLineIndent;
}
}
}
/// A diagnostic message. Each message defines one issue in the file, which
/// consists of one or more squiggly line ranges within the code, as well as
/// human readable descriptions and optionally also one or more automatic code
/// fixes that can be applied.
struct Message
{
/// A squiggly line range within the code. May be the issue itself if it's
/// the `diagnostic` member or supplemental information that can aid the
/// user in resolving the issue.
struct Diagnostic
{
/// Name of the file where the warning was triggered.
string fileName;
/// Byte index from start of file the warning was triggered.
size_t startIndex, endIndex;
/// Line number where the warning was triggered, 1-based.
size_t startLine, endLine;
/// Column number where the warning was triggered. (in bytes)
size_t startColumn, endColumn;
/// Warning message, may be null for supplemental diagnostics.
string message;
deprecated("Use startLine instead") alias line = startLine;
deprecated("Use startColumn instead") alias column = startColumn;
static Diagnostic from(string fileName, const BaseNode node, string message)
{
return from(fileName, node !is null ? node.tokens : [], message);
}
static Diagnostic from(string fileName, const Token token, string message)
{
auto text = token.text.length ? token.text : str(token.type);
return from(fileName,
[token.index, token.index + text.length],
token.line,
[token.column, token.column + text.length],
message);
}
static Diagnostic from(string fileName, const Token[] tokens, string message)
{
auto start = tokens.length ? tokens[0] : Token.init;
auto end = tokens.length ? tokens[$ - 1] : Token.init;
auto endText = end.text.length ? end.text : str(end.type);
return from(fileName,
[start.index, end.index + endText.length],
[start.line, end.line],
[start.column, end.column + endText.length],
message);
}
static Diagnostic from(string fileName, size_t[2] index, size_t line, size_t[2] columns, string message)
{
return Message.Diagnostic(fileName, index[0], index[1], line, line, columns[0], columns[1], message);
}
static Diagnostic from(string fileName, size_t[2] index, size_t[2] lines, size_t[2] columns, string message)
{
return Message.Diagnostic(fileName, index[0], index[1], lines[0], lines[1], columns[0], columns[1], message);
}
}
/// Primary warning
Diagnostic diagnostic;
/// List of supplemental warnings / hint locations
Diagnostic[] supplemental;
/// Name of the warning
string key;
/// Check name
string checkName;
/// Either immediate code changes that can be applied or context to call
/// the `BaseAnalyzer.resolveAutoFix` method with.
AutoFix[] autofixes;
deprecated this(string fileName, size_t line, size_t column, string key = null, string message = null, string checkName = null)
{
diagnostic.fileName = fileName;
diagnostic.startLine = diagnostic.endLine = line;
diagnostic.startColumn = diagnostic.endColumn = column;
diagnostic.message = message;
this.key = key;
this.checkName = checkName;
}
this(Diagnostic diagnostic, string key = null, string checkName = null, AutoFix[] autofixes = null)
{
this.diagnostic = diagnostic;
this.key = key;
this.checkName = checkName;
this.autofixes = autofixes;
}
this(Diagnostic diagnostic, Diagnostic[] supplemental, string key = null, string checkName = null, AutoFix[] autofixes = null)
{
this.diagnostic = diagnostic;
this.supplemental = supplemental;
this.key = key;
this.checkName = checkName;
this.autofixes = autofixes;
}
alias diagnostic this;
}
enum comparitor = q{ a.startLine < b.startLine || (a.startLine == b.startLine && a.startColumn < b.startColumn) };
alias MessageSet = RedBlackTree!(Message, comparitor, true);
@ -36,18 +371,45 @@ mixin template AnalyzerInfo(string checkName)
}
}
struct BaseAnalyzerArguments
{
string fileName;
const(Token)[] tokens;
const Scope* sc;
bool skipTests = false;
BaseAnalyzerArguments setSkipTests(bool v)
{
auto ret = this;
ret.skipTests = v;
return ret;
}
}
abstract class BaseAnalyzer : ASTVisitor
{
public:
deprecated("Don't use this constructor, use the one taking BaseAnalyzerArguments")
this(string fileName, const Scope* sc, bool skipTests = false)
{
this.sc = sc;
this.fileName = fileName;
this.skipTests = skipTests;
BaseAnalyzerArguments args = {
fileName: fileName,
sc: sc,
skipTests: skipTests
};
this(args);
}
this(BaseAnalyzerArguments args)
{
this.sc = args.sc;
this.tokens = args.tokens;
this.fileName = args.fileName;
this.skipTests = args.skipTests;
_messages = new MessageSet;
}
protected string getName()
string getName()
{
assert(0);
}
@ -71,10 +433,55 @@ public:
unittest_.accept(this);
}
/**
* Visits a module declaration.
*
* When overriden, make sure to keep this structure
*/
override void visit(const(Module) mod)
{
if (mod.moduleDeclaration !is null)
{
with (noLint.push(NoLintFactory.fromModuleDeclaration(mod.moduleDeclaration)))
mod.accept(this);
}
else
{
mod.accept(this);
}
}
/**
* Visits a declaration.
*
* When overriden, make sure to disable and reenable error messages
*/
override void visit(const(Declaration) decl)
{
with (noLint.push(NoLintFactory.fromDeclaration(decl)))
decl.accept(this);
}
AutoFix.CodeReplacement[] resolveAutoFix(
const Module mod,
scope const(Token)[] tokens,
const AutoFix.ResolveContext context,
const AutoFixFormatting formatting,
)
{
cast(void) mod;
cast(void) tokens;
cast(void) context;
cast(void) formatting;
assert(0);
}
protected:
bool inAggregate;
bool skipTests;
const(Token)[] tokens;
NoLint noLint;
template visitTemplate(T)
{
@ -86,11 +493,64 @@ protected:
}
}
deprecated("Use the overload taking start and end locations or a Node instead")
void addErrorMessage(size_t line, size_t column, string key, string message)
{
if (noLint.containsCheck(key))
return;
_messages.insert(Message(fileName, line, column, key, message, getName()));
}
void addErrorMessage(const BaseNode node, string key, string message, AutoFix[] autofixes = null)
{
if (noLint.containsCheck(key))
return;
addErrorMessage(Message.Diagnostic.from(fileName, node, message), key, autofixes);
}
void addErrorMessage(const Token token, string key, string message, AutoFix[] autofixes = null)
{
if (noLint.containsCheck(key))
return;
addErrorMessage(Message.Diagnostic.from(fileName, token, message), key, autofixes);
}
void addErrorMessage(const Token[] tokens, string key, string message, AutoFix[] autofixes = null)
{
if (noLint.containsCheck(key))
return;
addErrorMessage(Message.Diagnostic.from(fileName, tokens, message), key, autofixes);
}
void addErrorMessage(size_t[2] index, size_t line, size_t[2] columns, string key, string message, AutoFix[] autofixes = null)
{
if (noLint.containsCheck(key))
return;
addErrorMessage(index, [line, line], columns, key, message, autofixes);
}
void addErrorMessage(size_t[2] index, size_t[2] lines, size_t[2] columns, string key, string message, AutoFix[] autofixes = null)
{
if (noLint.containsCheck(key))
return;
auto d = Message.Diagnostic.from(fileName, index, lines, columns, message);
_messages.insert(Message(d, key, getName(), autofixes));
}
void addErrorMessage(Message.Diagnostic diagnostic, string key, AutoFix[] autofixes = null)
{
if (noLint.containsCheck(key))
return;
_messages.insert(Message(diagnostic, key, getName(), autofixes));
}
void addErrorMessage(Message.Diagnostic diagnostic, Message.Diagnostic[] supplemental, string key, AutoFix[] autofixes = null)
{
if (noLint.containsCheck(key))
return;
_messages.insert(Message(diagnostic, supplemental, key, getName(), autofixes));
}
/**
* The file name
*/
@ -100,3 +560,340 @@ protected:
MessageSet _messages;
}
/// Find the token with the given type, otherwise returns the whole range or a user-specified fallback, if set.
const(Token)[] findTokenForDisplay(const BaseNode node, IdType type, const(Token)[] fallback = null)
{
return node.tokens.findTokenForDisplay(type, fallback);
}
/// ditto
const(Token)[] findTokenForDisplay(const Token[] tokens, IdType type, const(Token)[] fallback = null)
{
foreach (i, token; tokens)
if (token.type == type)
return tokens[i .. i + 1];
return fallback is null ? tokens : fallback;
}
abstract class ScopedBaseAnalyzer : BaseAnalyzer
{
public:
this(BaseAnalyzerArguments args)
{
super(args);
}
template ScopedVisit(NodeType)
{
override void visit(const NodeType n)
{
pushScopeImpl();
scope (exit)
popScopeImpl();
n.accept(this);
}
}
alias visit = BaseAnalyzer.visit;
mixin ScopedVisit!BlockStatement;
mixin ScopedVisit!ForeachStatement;
mixin ScopedVisit!ForStatement;
mixin ScopedVisit!Module;
mixin ScopedVisit!StructBody;
mixin ScopedVisit!TemplateDeclaration;
mixin ScopedVisit!WithStatement;
mixin ScopedVisit!WhileStatement;
mixin ScopedVisit!DoStatement;
// mixin ScopedVisit!SpecifiedFunctionBody; // covered by BlockStatement
mixin ScopedVisit!ShortenedFunctionBody;
override void visit(const SwitchStatement switchStatement)
{
switchStack.length++;
scope (exit)
switchStack.length--;
switchStatement.accept(this);
}
override void visit(const IfStatement ifStatement)
{
pushScopeImpl();
if (ifStatement.condition)
ifStatement.condition.accept(this);
if (ifStatement.thenStatement)
ifStatement.thenStatement.accept(this);
popScopeImpl();
if (ifStatement.elseStatement)
{
pushScopeImpl();
ifStatement.elseStatement.accept(this);
popScopeImpl();
}
}
static foreach (T; AliasSeq!(CaseStatement, DefaultStatement, CaseRangeStatement))
override void visit(const T stmt)
{
// case and default statements always open new scopes and close
// previous case scopes
bool close = switchStack.length && switchStack[$ - 1].inCase;
bool b = switchStack[$ - 1].inCase;
switchStack[$ - 1].inCase = true;
scope (exit)
switchStack[$ - 1].inCase = b;
if (close)
{
popScope();
pushScope();
stmt.accept(this);
}
else
{
pushScope();
stmt.accept(this);
popScope();
}
}
protected:
/// Called on new scopes, which includes for example:
///
/// - `module m; /* here, entire file */`
/// - `{ /* here */ }`
/// - `if () { /* here */ } else { /* here */ }`
/// - `foreach (...) { /* here */ }`
/// - `case 1: /* here */ break;`
/// - `case 1: /* here, up to next case */ goto case; case 2: /* here 2 */ break;`
/// - `default: /* here */ break;`
/// - `struct S { /* here */ }`
///
/// But doesn't include:
///
/// - `static if (x) { /* not a separate scope */ }` (use `mixin ScopedVisit!ConditionalDeclaration;`)
///
/// You can `mixin ScopedVisit!NodeType` to automatically call push/popScope
/// on occurences of that NodeType.
abstract void pushScope();
/// ditto
abstract void popScope();
void pushScopeImpl()
{
if (switchStack.length)
switchStack[$ - 1].scopeDepth++;
pushScope();
}
void popScopeImpl()
{
if (switchStack.length)
switchStack[$ - 1].scopeDepth--;
popScope();
}
struct SwitchStack
{
int scopeDepth;
bool inCase;
}
SwitchStack[] switchStack;
}
unittest
{
import core.exception : AssertError;
import dparse.lexer : getTokensForParser, LexerConfig, StringCache;
import dparse.parser : parseModule;
import dparse.rollback_allocator : RollbackAllocator;
import std.conv : to;
import std.exception : assertThrown;
// test where we can:
// call `depth(1);` to check that the scope depth is at 1
// if calls are syntactically not valid, define `auto depth = 1;`
//
// call `isNewScope();` to check that the scope hasn't been checked with isNewScope before
// if calls are syntactically not valid, define `auto isNewScope = void;`
//
// call `isOldScope();` to check that the scope has already been checked with isNewScope
// if calls are syntactically not valid, define `auto isOldScope = void;`
class TestScopedAnalyzer : ScopedBaseAnalyzer
{
this(size_t codeLine)
{
super(BaseAnalyzerArguments("stdin"));
this.codeLine = codeLine;
}
override void visit(const FunctionCallExpression f)
{
int depth = cast(int) stack.length;
if (f.unaryExpression && f.unaryExpression.primaryExpression
&& f.unaryExpression.primaryExpression.identifierOrTemplateInstance)
{
auto fname = f.unaryExpression.primaryExpression.identifierOrTemplateInstance.identifier.text;
if (fname == "depth")
{
assert(f.arguments.tokens.length == 3);
auto expected = f.arguments.tokens[1].text.to!int;
assert(expected == depth, "Expected depth="
~ expected.to!string ~ " in line " ~ (codeLine + f.tokens[0].line).to!string
~ ", but got depth=" ~ depth.to!string);
}
else if (fname == "isNewScope")
{
assert(!stack[$ - 1]);
stack[$ - 1] = true;
}
else if (fname == "isOldScope")
{
assert(stack[$ - 1]);
}
}
}
override void visit(const AutoDeclarationPart p)
{
int depth = cast(int) stack.length;
if (p.identifier.text == "depth")
{
assert(p.initializer.tokens.length == 1);
auto expected = p.initializer.tokens[0].text.to!int;
assert(expected == depth, "Expected depth="
~ expected.to!string ~ " in line " ~ (codeLine + p.tokens[0].line).to!string
~ ", but got depth=" ~ depth.to!string);
}
else if (p.identifier.text == "isNewScope")
{
assert(!stack[$ - 1]);
stack[$ - 1] = true;
}
else if (p.identifier.text == "isOldScope")
{
assert(stack[$ - 1]);
}
}
override void pushScope()
{
stack.length++;
}
override void popScope()
{
stack.length--;
}
alias visit = ScopedBaseAnalyzer.visit;
bool[] stack;
size_t codeLine;
}
void testScopes(string code, size_t codeLine = __LINE__ - 1)
{
StringCache cache = StringCache(4096);
LexerConfig config;
RollbackAllocator rba;
auto tokens = getTokensForParser(code, config, &cache);
Module m = parseModule(tokens, "stdin", &rba);
auto analyzer = new TestScopedAnalyzer(codeLine);
analyzer.visit(m);
}
testScopes(q{
auto isNewScope = void;
auto depth = 1;
auto isOldScope = void;
});
assertThrown!AssertError(testScopes(q{
auto isNewScope = void;
auto isNewScope = void;
}));
assertThrown!AssertError(testScopes(q{
auto isOldScope = void;
}));
assertThrown!AssertError(testScopes(q{
auto depth = 2;
}));
testScopes(q{
auto isNewScope = void;
auto depth = 1;
void foo() {
isNewScope();
isOldScope();
depth(2);
switch (a)
{
case 1:
isNewScope();
depth(4);
break;
depth(4);
isOldScope();
case 2:
isNewScope();
depth(4);
if (a)
{
isNewScope();
depth(6);
default:
isNewScope();
depth(6); // since cases/default opens new scope
break;
case 3:
isNewScope();
depth(6); // since cases/default opens new scope
break;
default:
isNewScope();
depth(6); // since cases/default opens new scope
break;
}
break;
depth(4);
default:
isNewScope();
depth(4);
break;
depth(4);
}
isOldScope();
depth(2);
switch (a)
{
isNewScope();
depth(3);
isOldScope();
default:
isNewScope();
depth(4);
break;
isOldScope();
case 1:
isNewScope();
depth(4);
break;
isOldScope();
}
}
auto isOldScope = void;
});
}

View File

@ -0,0 +1,215 @@
module dscanner.analysis.body_on_disabled_funcs;
import dscanner.analysis.base;
import dparse.ast;
import dparse.lexer;
import dsymbol.scope_;
import std.meta : AliasSeq;
final class BodyOnDisabledFuncsCheck : BaseAnalyzer
{
alias visit = BaseAnalyzer.visit;
mixin AnalyzerInfo!"body_on_disabled_func_check";
this(BaseAnalyzerArguments args)
{
super(args);
}
static foreach (AggregateType; AliasSeq!(InterfaceDeclaration, ClassDeclaration,
StructDeclaration, UnionDeclaration, FunctionDeclaration))
override void visit(const AggregateType t)
{
scope wasDisabled = isDisabled;
isDisabled = false;
t.accept(this);
isDisabled = wasDisabled;
}
override void visit(const Declaration dec)
{
foreach (attr; dec.attributes)
{
if (attr.atAttribute !is null && attr.atAttribute.identifier.text == "disable") {
// found attr block w. disable: dec.constructor
scope wasDisabled = isDisabled;
isDisabled = true;
visitDeclarationInner(dec);
dec.accept(this);
isDisabled = wasDisabled;
return;
}
}
visitDeclarationInner(dec);
scope wasDisabled = isDisabled;
dec.accept(this);
isDisabled = wasDisabled;
}
private:
bool isDisabled = false;
bool isDeclDisabled(T)(const T dec)
{
// `@disable { ... }`
if (isDisabled)
return true;
static if (__traits(hasMember, T, "storageClasses"))
{
// `@disable doThing() {}`
if (hasDisabledStorageclass(dec.storageClasses))
return true;
}
// `void doThing() @disable {}`
return hasDisabledMemberFunctionAttribute(dec.memberFunctionAttributes);
}
void visitDeclarationInner(const Declaration dec)
{
if (dec.attributeDeclaration !is null
&& dec.attributeDeclaration.attribute
&& dec.attributeDeclaration.attribute.atAttribute
&& dec.attributeDeclaration.attribute.atAttribute.identifier.text == "disable")
{
// found `@disable:`, so all code in this block is now disabled
isDisabled = true;
}
else if (dec.functionDeclaration !is null
&& isDeclDisabled(dec.functionDeclaration)
&& dec.functionDeclaration.functionBody !is null
&& dec.functionDeclaration.functionBody.missingFunctionBody is null)
{
addErrorMessage(dec.functionDeclaration.functionBody,
KEY, "Function marked with '@disabled' should not have a body");
}
else if (dec.constructor !is null
&& isDeclDisabled(dec.constructor)
&& dec.constructor.functionBody !is null
&& dec.constructor.functionBody.missingFunctionBody is null)
{
addErrorMessage(dec.constructor.functionBody,
KEY, "Constructor marked with '@disabled' should not have a body");
}
else if (dec.destructor !is null
&& isDeclDisabled(dec.destructor)
&& dec.destructor.functionBody !is null
&& dec.destructor.functionBody.missingFunctionBody is null)
{
addErrorMessage(dec.destructor.functionBody,
KEY, "Destructor marked with '@disabled' should not have a body");
}
}
bool hasDisabledStorageclass(const(StorageClass[]) storageClasses)
{
foreach (sc; storageClasses)
{
if (sc.atAttribute !is null && sc.atAttribute.identifier.text == "disable")
return true;
}
return false;
}
bool hasDisabledMemberFunctionAttribute(const(MemberFunctionAttribute[]) memberFunctionAttributes)
{
foreach (attr; memberFunctionAttributes)
{
if (attr.atAttribute !is null && attr.atAttribute.identifier.text == "disable")
return true;
}
return false;
}
enum string KEY = "dscanner.confusing.disabled_function_with_body";
}
unittest
{
import std.stdio : stderr;
import std.format : format;
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings;
StaticAnalysisConfig sac = disabledConfig();
sac.body_on_disabled_func_check = Check.enabled;
assertAnalyzerWarnings(q{
class C1
{
this() {}
void doThing() {}
~this() {}
this();
void doThing();
~this();
@disable:
@disable
{
class UnaffectedSubClass
{
this() {}
void doThing() {}
~this() {}
}
}
this() {} /+
^^ [warn]: Constructor marked with '@disabled' should not have a body +/
void doThing() {} /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
~this() {} /+
^^ [warn]: Destructor marked with '@disabled' should not have a body +/
this();
void doThing();
~this();
}
class C2
{
@disable this() {} /+
^^ [warn]: Constructor marked with '@disabled' should not have a body +/
@disable { this() {} } /+
^^ [warn]: Constructor marked with '@disabled' should not have a body +/
this() @disable {} /+
^^ [warn]: Constructor marked with '@disabled' should not have a body +/
@disable void doThing() {} /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
@disable doThing() {} /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
@disable { void doThing() {} } /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
void doThing() @disable {} /+
^^ [warn]: Function marked with '@disabled' should not have a body +/
@disable ~this() {} /+
^^ [warn]: Destructor marked with '@disabled' should not have a body +/
@disable { ~this() {} } /+
^^ [warn]: Destructor marked with '@disabled' should not have a body +/
~this() @disable {} /+
^^ [warn]: Destructor marked with '@disabled' should not have a body +/
@disable this();
@disable { this(); }
this() @disable;
@disable void doThing();
// @disable doThing(); // this is invalid grammar!
@disable { void doThing(); }
void doThing() @disable;
@disable ~this();
@disable { ~this(); }
~this() @disable;
}
}c, sac);
stderr.writeln("Unittest for BodyOnDisabledFuncsCheck passed.");
}

View File

@ -33,16 +33,16 @@ final class BuiltinPropertyNameCheck : BaseAnalyzer
mixin AnalyzerInfo!"builtin_property_names_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const FunctionDeclaration fd)
{
if (depth > 0 && isBuiltinProperty(fd.name.text))
{
addErrorMessage(fd.name.line, fd.name.column, KEY, generateErrorMessage(fd.name.text));
addErrorMessage(fd.name, KEY, generateErrorMessage(fd.name.text));
}
fd.accept(this);
}
@ -62,14 +62,14 @@ final class BuiltinPropertyNameCheck : BaseAnalyzer
foreach (i; ad.parts.map!(a => a.identifier))
{
if (isBuiltinProperty(i.text))
addErrorMessage(i.line, i.column, KEY, generateErrorMessage(i.text));
addErrorMessage(i, KEY, generateErrorMessage(i.text));
}
}
override void visit(const Declarator d)
{
if (depth > 0 && isBuiltinProperty(d.name.text))
addErrorMessage(d.name.line, d.name.column, KEY, generateErrorMessage(d.name.text));
addErrorMessage(d.name, KEY, generateErrorMessage(d.name.text));
}
override void visit(const StructBody sb)
@ -111,9 +111,12 @@ unittest
assertAnalyzerWarnings(q{
class SomeClass
{
void init(); // [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type.
int init; // [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type.
auto init = 10; // [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type.
void init(); /+
^^^^ [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. +/
int init; /+
^^^^ [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. +/
auto init = 10; /+
^^^^ [warn]: Avoid naming members 'init'. This can confuse code that depends on the '.init' property of a type. +/
}
}c, sac);

View File

@ -19,16 +19,16 @@ final class CommaExpressionCheck : BaseAnalyzer
mixin AnalyzerInfo!"comma_expression_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const Expression ex)
{
if (ex.items.length > 1 && interest > 0)
{
addErrorMessage(ex.line, ex.column, KEY, "Avoid using the comma expression.");
addErrorMessage(ex, KEY, "Avoid using the comma expression.");
}
ex.accept(this);
}

View File

@ -165,7 +165,10 @@ struct StaticAnalysisConfig
string lambda_return_check = Check.enabled;
@INI("Check for auto function without return statement")
string auto_function_check = Check.enabled;
string auto_function_check = Check.disabled;
@INI("Check that if|else|for|foreach|while|do|try|catch are always followed by a BlockStatement { }")
string always_curly_check = Check.disabled;
@INI("Check for sortedness of imports")
string imports_sortedness = Check.disabled;
@ -215,8 +218,74 @@ struct StaticAnalysisConfig
@INI("Maximum cyclomatic complexity after which to issue warnings")
int max_cyclomatic_complexity = 50;
@INI("Check for function bodies on disabled functions")
string body_on_disabled_func_check = Check.enabled;
@INI("Module-specific filters")
ModuleFilters filters;
@INI("Formatting brace style for automatic fixes (allman, otbs, stroustrup, knr)")
string brace_style = "allman";
@INI("Formatting indentation style for automatic fixes (tab, space)")
string indentation_style = "tab";
@INI("Formatting indentation width for automatic fixes (space count, otherwise how wide a tab is)")
int indentation_width = 4;
@INI("Formatting line ending character (lf, cr, crlf)")
string eol_style = "lf";
auto getAutoFixFormattingConfig() const
{
import dscanner.analysis.base : AutoFixFormatting;
import std.array : array;
import std.conv : to;
import std.range : repeat;
if (indentation_width < 0)
throw new Exception("invalid negative indentation_width");
AutoFixFormatting ret;
ret.braceStyle = brace_style.to!(AutoFixFormatting.BraceStyle);
ret.indentationWidth = indentation_width;
switch (indentation_style)
{
case "tab":
ret.indentation = "\t";
break;
case "space":
static immutable string someSpaces = " ";
if (indentation_width < someSpaces.length)
ret.indentation = someSpaces[0 .. indentation_width];
else
ret.indentation = ' '.repeat(indentation_width).array;
break;
default:
throw new Exception("invalid indentation_style: '" ~ indentation_style ~ "' (expected tab or space)");
}
switch (eol_style)
{
case "lf":
case "LF":
ret.eol = "\n";
break;
case "cr":
case "CR":
ret.eol = "\r";
break;
case "crlf":
case "CRLF":
ret.eol = "\r\n";
break;
default:
throw new Exception("invalid eol_style: '" ~ eol_style ~ "' (expected lf, cr or crlf)");
}
return ret;
}
}
private template ModuleFiltersMixin(A)

View File

@ -3,6 +3,7 @@ module dscanner.analysis.constructors;
import dparse.ast;
import dparse.lexer;
import std.stdio;
import std.typecons : Rebindable;
import dscanner.analysis.base;
import dscanner.analysis.helpers;
import dsymbol.scope_ : Scope;
@ -13,26 +14,32 @@ final class ConstructorCheck : BaseAnalyzer
mixin AnalyzerInfo!"constructor_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const ClassDeclaration classDeclaration)
{
immutable bool oldHasDefault = hasDefaultArgConstructor;
immutable bool oldHasNoArg = hasNoArgConstructor;
hasNoArgConstructor = false;
hasDefaultArgConstructor = false;
const oldHasDefault = hasDefaultArgConstructor;
const oldHasNoArg = hasNoArgConstructor;
hasNoArgConstructor = null;
hasDefaultArgConstructor = null;
immutable State prev = state;
state = State.inClass;
classDeclaration.accept(this);
if (hasNoArgConstructor && hasDefaultArgConstructor)
{
addErrorMessage(classDeclaration.name.line,
classDeclaration.name.column, "dscanner.confusing.constructor_args",
addErrorMessage(
Message.Diagnostic.from(fileName, classDeclaration.name,
"This class has a zero-argument constructor as well as a"
~ " constructor with one default argument. This can be confusing.");
~ " constructor with one default argument. This can be confusing."),
[
Message.Diagnostic.from(fileName, hasNoArgConstructor, "zero-argument constructor defined here"),
Message.Diagnostic.from(fileName, hasDefaultArgConstructor, "default argument constructor defined here")
],
"dscanner.confusing.constructor_args"
);
}
hasDefaultArgConstructor = oldHasDefault;
hasNoArgConstructor = oldHasNoArg;
@ -55,7 +62,11 @@ final class ConstructorCheck : BaseAnalyzer
if (constructor.parameters.parameters.length == 1
&& constructor.parameters.parameters[0].default_ !is null)
{
addErrorMessage(constructor.line, constructor.column,
const(Token)[] tokens = constructor.parameters.parameters[0].default_.tokens;
assert(tokens.length);
// we extend the token range to the `=` sign, since it's continuous
tokens = (tokens.ptr - 1)[0 .. tokens.length + 1];
addErrorMessage(tokens,
"dscanner.confusing.struct_constructor_default_args",
"This struct constructor can never be called with its "
~ "default argument.");
@ -65,10 +76,10 @@ final class ConstructorCheck : BaseAnalyzer
if (constructor.parameters.parameters.length == 1
&& constructor.parameters.parameters[0].default_ !is null)
{
hasDefaultArgConstructor = true;
hasDefaultArgConstructor = constructor;
}
else if (constructor.parameters.parameters.length == 0)
hasNoArgConstructor = true;
hasNoArgConstructor = constructor;
break;
case State.ignoring:
break;
@ -86,8 +97,8 @@ private:
State state;
bool hasNoArgConstructor;
bool hasDefaultArgConstructor;
Rebindable!(const Constructor) hasNoArgConstructor;
Rebindable!(const Constructor) hasDefaultArgConstructor;
}
unittest
@ -96,8 +107,10 @@ unittest
StaticAnalysisConfig sac = disabledConfig();
sac.constructor_check = Check.enabled;
// TODO: test supplemental diagnostics
assertAnalyzerWarnings(q{
class Cat // [warn]: This class has a zero-argument constructor as well as a constructor with one default argument. This can be confusing.
class Cat /+
^^^ [warn]: This class has a zero-argument constructor as well as a constructor with one default argument. This can be confusing. +/
{
this() {}
this(string name = "kittie") {}
@ -106,7 +119,8 @@ unittest
struct Dog
{
this() {}
this(string name = "doggie") {} // [warn]: This struct constructor can never be called with its default argument.
this(string name = "doggie") {} /+
^^^^^^^^^^ [warn]: This struct constructor can never be called with its default argument. +/
}
}c, sac);

View File

@ -53,10 +53,9 @@ final class CyclomaticComplexityCheck : BaseAnalyzer
int maxCyclomaticComplexity;
///
this(string fileName, const(Scope)* sc, bool skipTests = false,
int maxCyclomaticComplexity = 50)
this(BaseAnalyzerArguments args, int maxCyclomaticComplexity = 50)
{
super(fileName, sc, skipTests);
super(args);
this.maxCyclomaticComplexity = maxCyclomaticComplexity;
}
@ -105,7 +104,7 @@ final class CyclomaticComplexityCheck : BaseAnalyzer
inLoop.length--;
}
fun.functionBody.accept(this);
testComplexity(fun.name.line, fun.name.column);
testComplexity(fun.name);
}
override void visit(const Unittest unittest_)
@ -120,7 +119,7 @@ final class CyclomaticComplexityCheck : BaseAnalyzer
inLoop.length--;
}
unittest_.accept(this);
testComplexity(unittest_.line, unittest_.column);
testComplexity(unittest_.findTokenForDisplay(tok!"unittest"));
}
}
@ -129,12 +128,12 @@ private:
int[] complexityStack = [0];
bool[] inLoop = [false];
void testComplexity(size_t line, size_t column)
void testComplexity(T)(T annotatable)
{
auto complexity = complexityStack[$ - 1];
if (complexity > maxCyclomaticComplexity)
{
addErrorMessage(line, column, KEY, format!MESSAGE(complexity));
addErrorMessage(annotatable, KEY, format!MESSAGE(complexity));
}
}
@ -169,24 +168,28 @@ unittest
sac.cyclomatic_complexity = Check.enabled;
sac.max_cyclomatic_complexity = 0;
assertAnalyzerWarnings(q{
unittest // [warn]: Cyclomatic complexity of this function is 1.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 1. +/
{
}
unittest // [warn]: Cyclomatic complexity of this function is 1.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 1. +/
{
writeln("hello");
writeln("world");
}
void main(string[] args) // [warn]: Cyclomatic complexity of this function is 3.
void main(string[] args) /+
^^^^ [warn]: Cyclomatic complexity of this function is 3. +/
{
if (!args.length)
return;
writeln("hello ", args);
}
unittest // [warn]: Cyclomatic complexity of this function is 1.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 1. +/
{
// static if / static foreach does not increase cyclomatic complexity
static if (stuff)
@ -194,7 +197,8 @@ unittest // [warn]: Cyclomatic complexity of this function is 1.
int a;
}
unittest // [warn]: Cyclomatic complexity of this function is 2.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 2. +/
{
foreach (i; 0 .. 2)
{
@ -202,7 +206,8 @@ unittest // [warn]: Cyclomatic complexity of this function is 2.
int a;
}
unittest // [warn]: Cyclomatic complexity of this function is 3.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 3. +/
{
foreach (i; 0 .. 2)
{
@ -211,7 +216,8 @@ unittest // [warn]: Cyclomatic complexity of this function is 3.
int a;
}
unittest // [warn]: Cyclomatic complexity of this function is 2.
unittest /+
^^^^^^^^ [warn]: Cyclomatic complexity of this function is 2. +/
{
switch (x)
{
@ -223,7 +229,8 @@ unittest // [warn]: Cyclomatic complexity of this function is 2.
int a;
}
bool shouldRun(check : BaseAnalyzer)( // [warn]: Cyclomatic complexity of this function is 20.
bool shouldRun(check : BaseAnalyzer)( /+
^^^^^^^^^ [warn]: Cyclomatic complexity of this function is 20. +/
string moduleName, const ref StaticAnalysisConfig config)
{
enum string a = check.name;

View File

@ -20,23 +20,27 @@ final class DeleteCheck : BaseAnalyzer
mixin AnalyzerInfo!"delete_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const DeleteExpression d)
{
addErrorMessage(d.line, d.column, "dscanner.deprecated.delete_keyword",
"Avoid using the 'delete' keyword.");
addErrorMessage(d.tokens[0], KEY,
"Avoid using the 'delete' keyword.",
[AutoFix.replacement(d.tokens[0], `destroy(`, "Replace delete with destroy()")
.concat(AutoFix.insertionAfter(d.tokens[$ - 1], ")"))]);
d.accept(this);
}
private enum string KEY = "dscanner.deprecated.delete_keyword";
}
unittest
{
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings;
import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
StaticAnalysisConfig sac = disabledConfig();
sac.delete_check = Check.enabled;
@ -44,10 +48,32 @@ unittest
void testDelete()
{
int[int] data = [1 : 2];
delete data[1]; // [warn]: Avoid using the 'delete' keyword.
delete data[1]; /+
^^^^^^ [warn]: Avoid using the 'delete' keyword. +/
auto a = new Class();
delete a; // [warn]: Avoid using the 'delete' keyword.
delete a; /+
^^^^^^ [warn]: Avoid using the 'delete' keyword. +/
}
}c, sac);
assertAutoFix(q{
void testDelete()
{
int[int] data = [1 : 2];
delete data[1]; // fix
auto a = new Class();
delete a; // fix
}
}c, q{
void testDelete()
{
int[int] data = [1 : 2];
destroy(data[1]); // fix
auto a = new Class();
destroy(a); // fix
}
}c, sac);

View File

@ -23,9 +23,9 @@ final class DuplicateAttributeCheck : BaseAnalyzer
mixin AnalyzerInfo!"duplicate_attribute";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const Declaration node)
@ -46,18 +46,18 @@ final class DuplicateAttributeCheck : BaseAnalyzer
// Check the attributes
foreach (attribute; node.attributes)
{
size_t line, column;
string attributeName = getAttributeName(attribute, line, column);
if (!attributeName || line == 0 || column == 0)
const(Token)[] tokens;
string attributeName = getAttributeName(attribute, tokens);
if (!attributeName || !tokens.length)
return;
// Check for the attributes
checkDuplicateAttribute(attributeName, "property", line, column, hasProperty);
checkDuplicateAttribute(attributeName, "safe", line, column, hasSafe);
checkDuplicateAttribute(attributeName, "trusted", line, column, hasTrusted);
checkDuplicateAttribute(attributeName, "system", line, column, hasSystem);
checkDuplicateAttribute(attributeName, "pure", line, column, hasPure);
checkDuplicateAttribute(attributeName, "nothrow", line, column, hasNoThrow);
checkDuplicateAttribute(attributeName, "property", tokens, hasProperty);
checkDuplicateAttribute(attributeName, "safe", tokens, hasSafe);
checkDuplicateAttribute(attributeName, "trusted", tokens, hasTrusted);
checkDuplicateAttribute(attributeName, "system", tokens, hasSystem);
checkDuplicateAttribute(attributeName, "pure", tokens, hasPure);
checkDuplicateAttribute(attributeName, "nothrow", tokens, hasNoThrow);
}
// Just return if missing function nodes
@ -67,23 +67,23 @@ final class DuplicateAttributeCheck : BaseAnalyzer
// Check the functions
foreach (memberFunctionAttribute; node.functionDeclaration.memberFunctionAttributes)
{
size_t line, column;
string attributeName = getAttributeName(memberFunctionAttribute, line, column);
if (!attributeName || line == 0 || column == 0)
const(Token)[] tokens;
string attributeName = getAttributeName(memberFunctionAttribute, tokens);
if (!attributeName || !tokens.length)
return;
// Check for the attributes
checkDuplicateAttribute(attributeName, "property", line, column, hasProperty);
checkDuplicateAttribute(attributeName, "safe", line, column, hasSafe);
checkDuplicateAttribute(attributeName, "trusted", line, column, hasTrusted);
checkDuplicateAttribute(attributeName, "system", line, column, hasSystem);
checkDuplicateAttribute(attributeName, "pure", line, column, hasPure);
checkDuplicateAttribute(attributeName, "nothrow", line, column, hasNoThrow);
checkDuplicateAttribute(attributeName, "property", tokens, hasProperty);
checkDuplicateAttribute(attributeName, "safe", tokens, hasSafe);
checkDuplicateAttribute(attributeName, "trusted", tokens, hasTrusted);
checkDuplicateAttribute(attributeName, "system", tokens, hasSystem);
checkDuplicateAttribute(attributeName, "pure", tokens, hasPure);
checkDuplicateAttribute(attributeName, "nothrow", tokens, hasNoThrow);
}
}
void checkDuplicateAttribute(const string attributeName,
const string attributeDesired, size_t line, size_t column, ref bool hasAttribute)
const string attributeDesired, const(Token)[] tokens, ref bool hasAttribute)
{
// Just return if not an attribute
if (attributeName != attributeDesired)
@ -93,14 +93,15 @@ final class DuplicateAttributeCheck : BaseAnalyzer
if (hasAttribute)
{
string message = "Attribute '%s' is duplicated.".format(attributeName);
addErrorMessage(line, column, "dscanner.unnecessary.duplicate_attribute", message);
addErrorMessage(tokens, KEY, message,
[AutoFix.replacement(tokens, "", "Remove second attribute " ~ attributeName)]);
}
// Mark it as having that attribute
hasAttribute = true;
}
string getAttributeName(const Attribute attribute, ref size_t line, ref size_t column)
string getAttributeName(const Attribute attribute, ref const(Token)[] outTokens)
{
// Get the name from the attribute identifier
if (attribute && attribute.atAttribute && attribute.atAttribute.identifier !is Token.init
@ -108,16 +109,14 @@ final class DuplicateAttributeCheck : BaseAnalyzer
&& attribute.atAttribute.identifier.text.length)
{
auto token = attribute.atAttribute.identifier;
line = token.line;
column = token.column;
outTokens = attribute.atAttribute.tokens;
return token.text;
}
// Get the attribute from the storage class token
if (attribute && attribute.attribute.type != tok!"")
{
line = attribute.attribute.line;
column = attribute.attribute.column;
outTokens = attribute.tokens;
return attribute.attribute.type.str;
}
@ -125,14 +124,14 @@ final class DuplicateAttributeCheck : BaseAnalyzer
}
string getAttributeName(const MemberFunctionAttribute memberFunctionAttribute,
ref size_t line, ref size_t column)
ref const(Token)[] outTokens)
{
// Get the name from the tokenType
if (memberFunctionAttribute && memberFunctionAttribute.tokenType !is IdType.init
&& memberFunctionAttribute.tokenType.str
&& memberFunctionAttribute.tokenType.str.length)
{
// FIXME: How do we get the line/column number?
outTokens = memberFunctionAttribute.tokens;
return memberFunctionAttribute.tokenType.str;
}
@ -144,13 +143,14 @@ final class DuplicateAttributeCheck : BaseAnalyzer
&& memberFunctionAttribute.atAttribute.identifier.text.length)
{
auto iden = memberFunctionAttribute.atAttribute.identifier;
line = iden.line;
column = iden.column;
outTokens = memberFunctionAttribute.atAttribute.tokens;
return iden.text;
}
return null;
}
private enum string KEY = "dscanner.unnecessary.duplicate_attribute";
}
unittest
@ -168,25 +168,29 @@ unittest
}
// Duplicate before
@property @property bool aaa() // [warn]: Attribute 'property' is duplicated.
@property @property bool aaa() /+
^^^^^^^^^ [warn]: Attribute 'property' is duplicated. +/
{
return false;
}
// Duplicate after
bool bbb() @safe @safe // [warn]: Attribute 'safe' is duplicated.
bool bbb() @safe @safe /+
^^^^^ [warn]: Attribute 'safe' is duplicated. +/
{
return false;
}
// Duplicate before and after
@system bool ccc() @system // [warn]: Attribute 'system' is duplicated.
@system bool ccc() @system /+
^^^^^^^ [warn]: Attribute 'system' is duplicated. +/
{
return false;
}
// Duplicate before and after
@trusted bool ddd() @trusted // [warn]: Attribute 'trusted' is duplicated.
@trusted bool ddd() @trusted /+
^^^^^^^^ [warn]: Attribute 'trusted' is duplicated. +/
{
return false;
}
@ -199,29 +203,66 @@ unittest
return false;
}
pure pure bool bbb() // [warn]: Attribute 'pure' is duplicated.
pure pure bool bbb() /+
^^^^ [warn]: Attribute 'pure' is duplicated. +/
{
return false;
}
// FIXME: There is no way to get the line/column number of the attribute like this
bool ccc() pure pure // FIXME: [warn]: Attribute 'pure' is duplicated.
bool ccc() pure pure /+
^^^^ [warn]: Attribute 'pure' is duplicated. +/
{
return false;
}
nothrow nothrow bool ddd() // [warn]: Attribute 'nothrow' is duplicated.
nothrow nothrow bool ddd() /+
^^^^^^^ [warn]: Attribute 'nothrow' is duplicated. +/
{
return false;
}
// FIXME: There is no way to get the line/column number of the attribute like this
bool eee() nothrow nothrow // FIXME: [warn]: Attribute 'nothrow' is duplicated.
bool eee() nothrow nothrow /+
^^^^^^^ [warn]: Attribute 'nothrow' is duplicated. +/
{
return false;
}
}
}c, sac);
assertAutoFix(q{
class ExampleAttributes
{
@property @property bool aaa() {} // fix
bool bbb() @safe @safe {} // fix
@system bool ccc() @system {} // fix
@trusted bool ddd() @trusted {} // fix
}
class ExamplePureNoThrow
{
pure pure bool bbb() {} // fix
bool ccc() pure pure {} // fix
nothrow nothrow bool ddd() {} // fix
bool eee() nothrow nothrow {} // fix
}
}c, q{
class ExampleAttributes
{
@property bool aaa() {} // fix
bool bbb() @safe {} // fix
@system bool ccc() {} // fix
@trusted bool ddd() {} // fix
}
class ExamplePureNoThrow
{
pure bool bbb() {} // fix
bool ccc() pure {} // fix
nothrow bool ddd() {} // fix
bool eee() nothrow {} // fix
}
}c, sac);
stderr.writeln("Unittest for DuplicateAttributeCheck passed.");
}

View File

@ -8,7 +8,7 @@ module dscanner.analysis.enumarrayliteral;
import dparse.ast;
import dparse.lexer;
import dscanner.analysis.base;
import std.algorithm : canFind, map;
import std.algorithm : find, map;
import dsymbol.scope_ : Scope;
void doNothing(string, size_t, size_t, string, bool)
@ -21,9 +21,9 @@ final class EnumArrayLiteralCheck : BaseAnalyzer
mixin AnalyzerInfo!"enum_array_literal_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
bool looking;
@ -35,7 +35,8 @@ final class EnumArrayLiteralCheck : BaseAnalyzer
override void visit(const AutoDeclaration autoDec)
{
if (autoDec.storageClasses.canFind!(a => a.token == tok!"enum"))
auto enumToken = autoDec.storageClasses.find!(a => a.token == tok!"enum");
if (enumToken.length)
{
foreach (part; autoDec.parts)
{
@ -45,13 +46,40 @@ final class EnumArrayLiteralCheck : BaseAnalyzer
continue;
if (part.initializer.nonVoidInitializer.arrayInitializer is null)
continue;
addErrorMessage(part.identifier.line, part.identifier.column,
"dscanner.performance.enum_array_literal",
addErrorMessage(part.initializer.nonVoidInitializer,
KEY,
"This enum may lead to unnecessary allocation at run-time."
~ " Use 'static immutable "
~ part.identifier.text ~ " = [ ...' instead.");
~ part.identifier.text ~ " = [ ...' instead.",
[
AutoFix.replacement(enumToken[0].token, "static immutable")
]);
}
}
autoDec.accept(this);
}
private enum string KEY = "dscanner.performance.enum_array_literal";
}
unittest
{
import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig();
sac.enum_array_literal_check = Check.enabled;
assertAnalyzerWarnings(q{
enum x = [1, 2, 3]; /+
^^^^^^^^^ [warn]: This enum may lead to unnecessary allocation at run-time. Use 'static immutable x = [ ...' instead. +/
}c, sac);
assertAutoFix(q{
enum x = [1, 2, 3]; // fix
}c, q{
static immutable x = [1, 2, 3]; // fix
}c, sac);
stderr.writeln("Unittest for EnumArrayLiteralCheck passed.");
}

View File

@ -20,9 +20,9 @@ final class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer
mixin AnalyzerInfo!"explicitly_annotated_unittests";
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const Declaration decl)
@ -44,7 +44,14 @@ final class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer
}
}
if (!isSafeOrSystem)
addErrorMessage(decl.unittest_.line, decl.unittest_.column, KEY, MESSAGE);
{
auto token = decl.unittest_.findTokenForDisplay(tok!"unittest");
addErrorMessage(token, KEY, MESSAGE,
[
AutoFix.insertionBefore(token[0], "@safe ", "Mark unittest @safe"),
AutoFix.insertionBefore(token[0], "@system ", "Mark unittest @system")
]);
}
}
decl.accept(this);
}
@ -55,10 +62,10 @@ final class ExplicitlyAnnotatedUnittestCheck : BaseAnalyzer
unittest
{
import std.stdio : stderr;
import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
import std.format : format;
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig();
sac.explicitly_annotated_unittests = Check.enabled;
@ -68,8 +75,10 @@ unittest
@system unittest {}
pure nothrow @system @nogc unittest {}
unittest {} // [warn]: %s
pure nothrow @nogc unittest {} // [warn]: %s
unittest {} /+
^^^^^^^^ [warn]: %s +/
pure nothrow @nogc unittest {} /+
^^^^^^^^ [warn]: %s +/
}c.format(
ExplicitlyAnnotatedUnittestCheck.MESSAGE,
ExplicitlyAnnotatedUnittestCheck.MESSAGE,
@ -82,13 +91,37 @@ unittest
@safe unittest {}
@system unittest {}
unittest {} // [warn]: %s
pure nothrow @nogc unittest {} // [warn]: %s
unittest {} /+
^^^^^^^^ [warn]: %s +/
pure nothrow @nogc unittest {} /+
^^^^^^^^ [warn]: %s +/
}
}c.format(
ExplicitlyAnnotatedUnittestCheck.MESSAGE,
ExplicitlyAnnotatedUnittestCheck.MESSAGE,
), sac);
// nested
assertAutoFix(q{
unittest {} // fix:0
pure nothrow @nogc unittest {} // fix:0
struct Foo
{
unittest {} // fix:1
pure nothrow @nogc unittest {} // fix:1
}
}c, q{
@safe unittest {} // fix:0
pure nothrow @nogc @safe unittest {} // fix:0
struct Foo
{
@system unittest {} // fix:1
pure nothrow @nogc @system unittest {} // fix:1
}
}c, sac);
stderr.writeln("Unittest for ExplicitlyAnnotatedUnittestCheck passed.");
}

View File

@ -54,12 +54,11 @@ private:
bool _blockStatic;
Parent _parent = Parent.module_;
void addError(T)(T t, string msg)
void addError(T)(const Token finalToken, T t, string msg)
{
import std.format : format;
const size_t lne = t.name.line;
const size_t col = t.name.column;
addErrorMessage(lne, col, KEY, MSGB.format(msg));
addErrorMessage(finalToken.type ? finalToken : t.name, KEY, MSGB.format(msg),
[AutoFix.replacement(finalToken, "")]);
}
public:
@ -75,9 +74,9 @@ public:
};
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const(StructDeclaration) sd)
@ -178,6 +177,11 @@ public:
const bool isFinal = d.attributes
.canFind!(a => a.attribute.type == tok!"final");
const Token finalToken = isFinal
? d.attributes
.filter!(a => a.attribute.type == tok!"final")
.front.attribute
: Token.init;
const bool isStaticOnce = d.attributes
.canFind!(a => a.attribute.type == tok!"static");
@ -203,9 +207,9 @@ public:
if (_finalAggregate && savedParent == Parent.module_)
{
if (d.structDeclaration)
addError(d.structDeclaration, MESSAGE.struct_i);
addError(finalToken, d.structDeclaration, MESSAGE.struct_i);
else if (d.unionDeclaration)
addError(d.unionDeclaration, MESSAGE.union_i);
addError(finalToken, d.unionDeclaration, MESSAGE.union_i);
}
}
@ -220,29 +224,29 @@ public:
{
case Parent.class_:
if (fd.templateParameters)
addError(fd, MESSAGE.class_t);
addError(finalToken, fd, MESSAGE.class_t);
if (isPrivate)
addError(fd, MESSAGE.class_p);
addError(finalToken, fd, MESSAGE.class_p);
else if (isStaticOnce || _alwaysStatic || _blockStatic)
addError(fd, MESSAGE.class_s);
addError(finalToken, fd, MESSAGE.class_s);
else if (_finalAggregate)
addError(fd, MESSAGE.class_f);
addError(finalToken, fd, MESSAGE.class_f);
break;
case Parent.interface_:
if (fd.templateParameters)
addError(fd, MESSAGE.interface_t);
addError(finalToken, fd, MESSAGE.interface_t);
break;
case Parent.struct_:
addError(fd, MESSAGE.struct_f);
addError(finalToken, fd, MESSAGE.struct_f);
break;
case Parent.union_:
addError(fd, MESSAGE.union_f);
addError(finalToken, fd, MESSAGE.union_f);
break;
case Parent.function_:
addError(fd, MESSAGE.func_n);
addError(finalToken, fd, MESSAGE.func_n);
break;
case Parent.module_:
addError(fd, MESSAGE.func_g);
addError(finalToken, fd, MESSAGE.func_g);
break;
}
}
@ -250,10 +254,10 @@ public:
@system unittest
{
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings;
import std.stdio : stderr;
import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
import std.format : format;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig();
sac.final_attribute_check = Check.enabled;
@ -317,13 +321,15 @@ public:
// fail
assertAnalyzerWarnings(q{
final void foo(){} // [warn]: %s
final void foo(){} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.func_g)
), sac);
assertAnalyzerWarnings(q{
void foo(){final void foo(){}} // [warn]: %s
void foo(){final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.func_n)
), sac);
@ -332,56 +338,65 @@ public:
void foo()
{
static if (true)
final class A{ private: final protected void foo(){}} // [warn]: %s
final class A{ private: final protected void foo(){}} /+
^^^^^ [warn]: %s +/
}
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_f)
), sac);
assertAnalyzerWarnings(q{
final struct Foo{} // [warn]: %s
final struct Foo{} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.struct_i)
), sac);
assertAnalyzerWarnings(q{
final union Foo{} // [warn]: %s
final union Foo{} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.union_i)
), sac);
assertAnalyzerWarnings(q{
class Foo{private final void foo(){}} // [warn]: %s
class Foo{private final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_p)
), sac);
assertAnalyzerWarnings(q{
class Foo{private: final void foo(){}} // [warn]: %s
class Foo{private: final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_p)
), sac);
assertAnalyzerWarnings(q{
interface Foo{final void foo(T)(){}} // [warn]: %s
interface Foo{final void foo(T)(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.interface_t)
), sac);
assertAnalyzerWarnings(q{
final class Foo{final void foo(){}} // [warn]: %s
final class Foo{final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_f)
), sac);
assertAnalyzerWarnings(q{
private: final class Foo {public: private final void foo(){}} // [warn]: %s
private: final class Foo {public: private final void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_p)
), sac);
assertAnalyzerWarnings(q{
class Foo {final static void foo(){}} // [warn]: %s
class Foo {final static void foo(){}} /+
^^^^^ [warn]: %s +/
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_s)
), sac);
@ -390,7 +405,8 @@ public:
class Foo
{
void foo(){}
static: final void foo(){} // [warn]: %s
static: final void foo(){} /+
^^^^^ [warn]: %s +/
}
}c.format(
FinalAttributeChecker.MSGB.format(FinalAttributeChecker.MESSAGE.class_s)
@ -400,7 +416,8 @@ public:
class Foo
{
void foo(){}
static{ final void foo(){}} // [warn]: %s
static{ final void foo(){}} /+
^^^^^ [warn]: %s +/
void foo(){}
}
}c.format(
@ -416,5 +433,62 @@ public:
}
}, sac);
assertAutoFix(q{
final void foo(){} // fix
void foo(){final void foo(){}} // fix
void foo()
{
static if (true)
final class A{ private: final protected void foo(){}} // fix
}
final struct Foo{} // fix
final union Foo{} // fix
class Foo{private final void foo(){}} // fix
class Foo{private: final void foo(){}} // fix
interface Foo{final void foo(T)(){}} // fix
final class Foo{final void foo(){}} // fix
private: final class Foo {public: private final void foo(){}} // fix
class Foo {final static void foo(){}} // fix
class Foo
{
void foo(){}
static: final void foo(){} // fix
}
class Foo
{
void foo(){}
static{ final void foo(){}} // fix
void foo(){}
}
}, q{
void foo(){} // fix
void foo(){ void foo(){}} // fix
void foo()
{
static if (true)
final class A{ private: protected void foo(){}} // fix
}
struct Foo{} // fix
union Foo{} // fix
class Foo{private void foo(){}} // fix
class Foo{private: void foo(){}} // fix
interface Foo{ void foo(T)(){}} // fix
final class Foo{ void foo(){}} // fix
private: final class Foo {public: private void foo(){}} // fix
class Foo { static void foo(){}} // fix
class Foo
{
void foo(){}
static: void foo(){} // fix
}
class Foo
{
void foo(){}
static{ void foo(){}} // fix
void foo(){}
}
}, sac);
stderr.writeln("Unittest for FinalAttributeChecker passed.");
}

View File

@ -22,9 +22,9 @@ final class FloatOperatorCheck : BaseAnalyzer
enum string KEY = "dscanner.deprecated.floating_point_operators";
mixin AnalyzerInfo!"float_operator_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const RelExpression r)
@ -34,7 +34,7 @@ final class FloatOperatorCheck : BaseAnalyzer
|| r.operator == tok!"!<" || r.operator == tok!"!<>="
|| r.operator == tok!"!>=" || r.operator == tok!"!<=")
{
addErrorMessage(r.line, r.column, KEY,
addErrorMessage(r, KEY,
"Avoid using the deprecated floating-point operators.");
}
r.accept(this);
@ -52,14 +52,22 @@ unittest
{
float z = 1.5f;
bool a;
a = z !<>= z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !<> z; // [warn]: Avoid using the deprecated floating-point operators.
a = z <> z; // [warn]: Avoid using the deprecated floating-point operators.
a = z <>= z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !> z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !>= z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !< z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !<= z; // [warn]: Avoid using the deprecated floating-point operators.
a = z !<>= z; /+
^^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !<> z; /+
^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z <> z; /+
^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z <>= z; /+
^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !> z; /+
^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !>= z; /+
^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !< z; /+
^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
a = z !<= z; /+
^^^^^^^ [warn]: Avoid using the deprecated floating-point operators. +/
}
}c, sac);

View File

@ -6,6 +6,7 @@
module dscanner.analysis.function_attributes;
import dscanner.analysis.base;
import dscanner.analysis.helpers;
import dparse.ast;
import dparse.lexer;
import std.stdio;
@ -27,39 +28,66 @@ final class FunctionAttributeCheck : BaseAnalyzer
mixin AnalyzerInfo!"function_attribute_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const InterfaceDeclaration dec)
{
const t = inInterface;
const t2 = inAggregate;
inInterface = true;
inAggregate = true;
dec.accept(this);
inInterface = t;
inAggregate = t2;
}
override void visit(const ClassDeclaration dec)
{
const t = inInterface;
const t2 = inAggregate;
inInterface = false;
inAggregate = true;
dec.accept(this);
inInterface = t;
inAggregate = t2;
}
override void visit(const StructDeclaration dec)
{
const t = inInterface;
const t2 = inAggregate;
inInterface = false;
inAggregate = true;
dec.accept(this);
inInterface = t;
inAggregate = t2;
}
override void visit(const UnionDeclaration dec)
{
const t = inInterface;
const t2 = inAggregate;
inInterface = false;
inAggregate = true;
dec.accept(this);
inInterface = t;
inAggregate = t2;
}
override void visit(const AttributeDeclaration dec)
{
if (inInterface && dec.attribute.attribute == tok!"abstract")
{
addErrorMessage(dec.attribute.attribute.line,
dec.attribute.attribute.column, KEY, ABSTRACT_MESSAGE);
addErrorMessage(dec.attribute, KEY, ABSTRACT_MESSAGE);
}
}
override void visit(const FunctionDeclaration dec)
{
if (dec.parameters.parameters.length == 0)
if (dec.parameters.parameters.length == 0 && inAggregate)
{
bool foundConst;
bool foundProperty;
@ -76,9 +104,15 @@ final class FunctionAttributeCheck : BaseAnalyzer
}
if (foundProperty && !foundConst)
{
addErrorMessage(dec.name.line, dec.name.column, KEY,
auto paren = dec.parameters.tokens.length ? dec.parameters.tokens[$ - 1] : Token.init;
auto autofixes = paren is Token.init ? null : [
AutoFix.insertionAfter(paren, " const", "Mark function `const`"),
AutoFix.insertionAfter(paren, " inout", "Mark function `inout`"),
AutoFix.insertionAfter(paren, " immutable", "Mark function `immutable`"),
];
addErrorMessage(dec.name, KEY,
"Zero-parameter '@property' function should be"
~ " marked 'const', 'inout', or 'immutable'.");
~ " marked 'const', 'inout', or 'immutable'.", autofixes);
}
}
dec.accept(this);
@ -86,6 +120,7 @@ final class FunctionAttributeCheck : BaseAnalyzer
override void visit(const Declaration dec)
{
bool isStatic = false;
if (dec.attributes.length == 0)
goto end;
foreach (attr; dec.attributes)
@ -94,27 +129,152 @@ final class FunctionAttributeCheck : BaseAnalyzer
continue;
if (attr.attribute == tok!"abstract" && inInterface)
{
addErrorMessage(attr.attribute.line, attr.attribute.column, KEY, ABSTRACT_MESSAGE);
addErrorMessage(attr.attribute, KEY, ABSTRACT_MESSAGE,
[AutoFix.replacement(attr.attribute, "")]);
continue;
}
if (attr.attribute == tok!"static")
{
isStatic = true;
}
if (dec.functionDeclaration !is null && (attr.attribute == tok!"const"
|| attr.attribute == tok!"inout" || attr.attribute == tok!"immutable"))
{
import std.string : format;
immutable string attrString = str(attr.attribute.type);
addErrorMessage(dec.functionDeclaration.name.line,
dec.functionDeclaration.name.column, KEY, format(
"'%s' is not an attribute of the return type." ~ " Place it after the parameter list to clarify.",
attrString));
AutoFix[] autofixes;
if (dec.functionDeclaration.parameters)
autofixes ~= AutoFix.replacement(
attr.attribute, "",
"Move " ~ str(attr.attribute.type) ~ " after parameter list")
.concat(AutoFix.insertionAfter(
dec.functionDeclaration.parameters.tokens[$ - 1],
" " ~ str(attr.attribute.type)));
if (dec.functionDeclaration.returnType)
autofixes ~= AutoFix.insertionAfter(attr.attribute, "(", "Make return type const")
.concat(AutoFix.insertionAfter(dec.functionDeclaration.returnType.tokens[$ - 1], ")"));
addErrorMessage(attr.attribute, KEY, format(
"'%s' is not an attribute of the return type."
~ " Place it after the parameter list to clarify.",
attrString), autofixes);
}
}
end:
dec.accept(this);
if (isStatic) {
const t = inAggregate;
inAggregate = false;
dec.accept(this);
inAggregate = t;
}
else {
dec.accept(this);
}
}
private:
bool inInterface;
bool inAggregate;
enum string ABSTRACT_MESSAGE = "'abstract' attribute is redundant in interface declarations";
enum string KEY = "dscanner.confusing.function_attributes";
}
unittest
{
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
StaticAnalysisConfig sac = disabledConfig();
sac.function_attribute_check = Check.enabled;
assertAnalyzerWarnings(q{
int foo() @property { return 0; }
class ClassName {
const int confusingConst() { return 0; } /+
^^^^^ [warn]: 'const' is not an attribute of the return type. Place it after the parameter list to clarify. +/
int bar() @property { return 0; } /+
^^^ [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'. +/
static int barStatic() @property { return 0; }
int barConst() const @property { return 0; }
}
struct StructName {
int bar() @property { return 0; } /+
^^^ [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'. +/
static int barStatic() @property { return 0; }
int barConst() const @property { return 0; }
}
union UnionName {
int bar() @property { return 0; } /+
^^^ [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'. +/
static int barStatic() @property { return 0; }
int barConst() const @property { return 0; }
}
interface InterfaceName {
int bar() @property; /+
^^^ [warn]: Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'. +/
static int barStatic() @property { return 0; }
int barConst() const @property;
abstract int method(); /+
^^^^^^^^ [warn]: 'abstract' attribute is redundant in interface declarations +/
}
}c, sac);
assertAutoFix(q{
int foo() @property { return 0; }
class ClassName {
const int confusingConst() { return 0; } // fix:0
const int confusingConst() { return 0; } // fix:1
int bar() @property { return 0; } // fix:0
int bar() @property { return 0; } // fix:1
int bar() @property { return 0; } // fix:2
}
struct StructName {
int bar() @property { return 0; } // fix:0
}
union UnionName {
int bar() @property { return 0; } // fix:0
}
interface InterfaceName {
int bar() @property; // fix:0
abstract int method(); // fix
}
}c, q{
int foo() @property { return 0; }
class ClassName {
int confusingConst() const { return 0; } // fix:0
const(int) confusingConst() { return 0; } // fix:1
int bar() const @property { return 0; } // fix:0
int bar() inout @property { return 0; } // fix:1
int bar() immutable @property { return 0; } // fix:2
}
struct StructName {
int bar() const @property { return 0; } // fix:0
}
union UnionName {
int bar() const @property { return 0; } // fix:0
}
interface InterfaceName {
int bar() const @property; // fix:0
int method(); // fix
}
}c, sac);
stderr.writeln("Unittest for FunctionAttributeCheck passed.");
}

View File

@ -22,9 +22,9 @@ final class HasPublicExampleCheck : BaseAnalyzer
mixin AnalyzerInfo!"has_public_example";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const Module mod)
@ -88,6 +88,8 @@ final class HasPublicExampleCheck : BaseAnalyzer
private:
enum string KEY = "dscanner.style.has_public_example";
bool hasDitto(Decl)(const Decl decl)
{
import ddoc.comments : parseComment;
@ -157,14 +159,14 @@ private:
{
foreach (property; possibleDeclarations)
if (auto fn = mixin("decl." ~ property))
addMessage(fn.name.line, fn.name.column, fn.name.text);
addMessage(fn.name.type ? [fn.name] : fn.tokens, fn.name.text);
}
void addMessage(size_t line, size_t column, string name)
void addMessage(const Token[] tokens, string name)
{
import std.string : format;
addErrorMessage(line, column, "dscanner.style.has_public_example", name is null
addErrorMessage(tokens, KEY, name is null
? "Public declaration has no documented example."
: format("Public declaration '%s' has no documented example.", name));
}
@ -220,27 +222,33 @@ unittest
// enums or variables don't need to have public unittest
assertAnalyzerWarnings(q{
/// C
class C{} // [warn]: Public declaration 'C' has no documented example.
class C{} /+
^ [warn]: Public declaration 'C' has no documented example. +/
unittest {}
/// I
interface I{} // [warn]: Public declaration 'I' has no documented example.
interface I{} /+
^ [warn]: Public declaration 'I' has no documented example. +/
unittest {}
/// f
void f(){} // [warn]: Public declaration 'f' has no documented example.
void f(){} /+
^ [warn]: Public declaration 'f' has no documented example. +/
unittest {}
/// S
struct S{} // [warn]: Public declaration 'S' has no documented example.
struct S{} /+
^ [warn]: Public declaration 'S' has no documented example. +/
unittest {}
/// T
template T(){} // [warn]: Public declaration 'T' has no documented example.
template T(){} /+
^ [warn]: Public declaration 'T' has no documented example. +/
unittest {}
/// U
union U{} // [warn]: Public declaration 'U' has no documented example.
union U{} /+
^ [warn]: Public declaration 'U' has no documented example. +/
unittest {}
}, sac);
@ -248,7 +256,8 @@ unittest
assertAnalyzerWarnings(q{
unittest {}
/// C
class C{} // [warn]: Public declaration 'C' has no documented example.
class C{} /+
^ [warn]: Public declaration 'C' has no documented example. +/
}, sac);
// test documented module header unittest
@ -256,13 +265,15 @@ unittest
///
unittest {}
/// C
class C{} // [warn]: Public declaration 'C' has no documented example.
class C{} /+
^ [warn]: Public declaration 'C' has no documented example. +/
}, sac);
// test multiple unittest blocks
assertAnalyzerWarnings(q{
/// C
class C{} // [warn]: Public declaration 'C' has no documented example.
class C{} /+
^ [warn]: Public declaration 'C' has no documented example. +/
unittest {}
unittest {}
unittest {}
@ -318,7 +329,8 @@ unittest
// test reset on private symbols (#500)
assertAnalyzerWarnings(q{
///
void dirName(C)(C[] path) {} // [warn]: Public declaration 'dirName' has no documented example.
void dirName(C)(C[] path) {} /+
^^^^^^^ [warn]: Public declaration 'dirName' has no documented example. +/
private void _dirName(R)(R path) {}
///
unittest {}

View File

@ -6,18 +6,19 @@
module dscanner.analysis.helpers;
import core.exception : AssertError;
import std.stdio;
import std.string;
import std.traits;
import std.stdio;
import dparse.ast;
import dparse.lexer : tok, Token;
import dparse.rollback_allocator;
import dsymbol.modulecache : ModuleCache;
import dscanner.analysis.base;
import dscanner.analysis.config;
import dscanner.analysis.run;
import dscanner.analysis.base;
import std.experimental.allocator.mallocator;
import dsymbol.modulecache : ModuleCache;
import std.experimental.allocator;
import std.experimental.allocator.mallocator;
S between(S)(S value, S before, S after) if (isSomeString!S)
{
@ -40,10 +41,32 @@ S after(S)(S value, S separator) if (isSomeString!S)
return value[i + separator.length .. $];
}
string getLineIndentation(scope const(Token)[] tokens, size_t line, const AutoFixFormatting formatting)
{
import std.algorithm : countUntil;
import std.array : array;
import std.range : repeat;
import std.string : lastIndexOfAny;
auto idx = tokens.countUntil!(a => a.line == line);
if (idx == -1 || tokens[idx].column <= 1 || !formatting.indentation.length)
return "";
auto indent = tokens[idx].column - 1;
if (formatting.indentation[0] == '\t')
return (cast(immutable)'\t').repeat(indent).array;
else
return (cast(immutable)' ').repeat(indent).array;
}
/**
* This assert function will analyze the passed in code, get the warnings,
* and make sure they match the warnings in the comments. Warnings are
* marked like so: // [warn]: Failed to do somethings.
* marked like so if range doesn't matter: // [warn]: Failed to do somethings.
*
* To test for start and end column, mark warnings as multi-line comments like
* this: /+
* ^^^^^ [warn]: Failed to do somethings. +/
*/
void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
string file = __FILE__, size_t line = __LINE__)
@ -62,12 +85,18 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
MessageSet rawWarnings = analyze("test", m, config, moduleCache, tokens);
string[] codeLines = code.splitLines();
struct FoundWarning
{
string msg;
size_t startColumn, endColumn;
}
// Get the warnings ordered by line
string[size_t] warnings;
FoundWarning[size_t] warnings;
foreach (rawWarning; rawWarnings[])
{
// Skip the warning if it is on line zero
immutable size_t rawLine = rawWarning.line;
immutable size_t rawLine = rawWarning.endLine;
if (rawLine == 0)
{
stderr.writefln("!!! Skipping warning because it is on line zero:\n%s",
@ -76,28 +105,49 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
}
size_t warnLine = line - 1 + rawLine;
warnings[warnLine] = format("[warn]: %s", rawWarning.message);
warnings[warnLine] = FoundWarning(
format("[warn]: %s", rawWarning.message),
rawWarning.startLine != rawWarning.endLine ? 1 : rawWarning.startColumn,
rawWarning.endColumn,
);
}
// Get all the messages from the comments in the code
string[size_t] messages;
FoundWarning[size_t] messages;
bool lastLineStartedComment = false;
foreach (i, codeLine; codeLines)
{
// Skip if no [warn] comment
if (codeLine.indexOf("// [warn]:") == -1)
continue;
// Skip if there is no comment or code
immutable string codePart = codeLine.before("// ");
immutable string commentPart = codeLine.after("// ");
if (!codePart.length || !commentPart.length)
continue;
scope (exit)
lastLineStartedComment = codeLine.stripRight.endsWith("/+", "/*") > 0;
// Get the line of this code line
size_t lineNo = i + line;
// Get the message
messages[lineNo] = commentPart;
if (codeLine.stripLeft.startsWith("^") && lastLineStartedComment)
{
auto start = codeLine.indexOf("^") + 1;
assert(start != 0);
auto end = codeLine.indexOfNeither("^", start) + 1;
assert(end != 0);
auto warn = codeLine.indexOf("[warn]:");
assert(warn != -1, "malformed line, expected `[warn]: text` after `^^^^^` part");
auto message = codeLine[warn .. $].stripRight;
if (message.endsWith("+/", "*/"))
message = message[0 .. $ - 2].stripRight;
messages[lineNo - 1] = FoundWarning(message, start, end);
}
// Skip if no [warn] comment
else if (codeLine.indexOf("// [warn]:") != -1)
{
// Skip if there is no comment or code
immutable string codePart = codeLine.before("// ");
immutable string commentPart = codeLine.after("// ");
if (!codePart.length || !commentPart.length)
continue;
// Get the message
messages[lineNo] = FoundWarning(commentPart);
}
}
// Throw an assert error if any messages are not listed in the warnings
@ -111,12 +161,39 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
throw new AssertError(errors, file, lineNo);
}
// Different warning
else if (warnings[lineNo] != messages[lineNo])
else if (warnings[lineNo].msg != messages[lineNo].msg)
{
immutable string errors = "Expected warning:\n%s\nBut was:\n%s\nFrom source code at (%s:?):\n%s".format(
messages[lineNo], warnings[lineNo], lineNo, codeLines[lineNo - line]);
throw new AssertError(errors, file, lineNo);
}
// specified column range
if ((message.startColumn || message.endColumn)
&& warnings[lineNo] != message)
{
import std.algorithm : max;
import std.array : array;
import std.range : repeat;
import std.string : replace;
const(char)[] expectedRange = ' '.repeat(max(0, cast(int)message.startColumn - 1)).array
~ '^'.repeat(max(0, cast(int)(message.endColumn - message.startColumn))).array;
const(char)[] actualRange;
if (!warnings[lineNo].startColumn || warnings[lineNo].startColumn == warnings[lineNo].endColumn)
actualRange = "no column range defined!";
else
actualRange = ' '.repeat(max(0, cast(int)warnings[lineNo].startColumn - 1)).array
~ '^'.repeat(max(0, cast(int)(warnings[lineNo].endColumn - warnings[lineNo].startColumn))).array;
size_t paddingWidth = max(expectedRange.length, actualRange.length);
immutable string errors = "Wrong warning range: expected %s, but was %s\nFrom source code at (%s:?):\n%s\n%-*s <-- expected\n%-*s <-- actual".format(
[message.startColumn, message.endColumn],
[warnings[lineNo].startColumn, warnings[lineNo].endColumn],
lineNo, codeLines[lineNo - line].replace("\t", " "),
paddingWidth, expectedRange,
paddingWidth, actualRange);
throw new AssertError(errors, file, lineNo);
}
}
// Throw an assert error if there were any warnings that were not expected
@ -136,3 +213,140 @@ void assertAnalyzerWarnings(string code, const StaticAnalysisConfig config,
throw new AssertError(message, file, line);
}
}
/// EOL inside this project, for tests
private static immutable fileEol = q{
};
/**
* This assert function will analyze the passed in code, get the warnings, and
* apply all specified autofixes all at once.
*
* Indicate which autofix to apply by adding a line comment at the end of the
* line with the following content: `// fix:0`, where 0 is the index which
* autofix to apply. There may only be one diagnostic on a line with this fix
* comment. Alternatively you can also just write `// fix` to apply the only
* available suggestion.
*/
void assertAutoFix(string before, string after, const StaticAnalysisConfig config,
const AutoFixFormatting formattingConfig = AutoFixFormatting(AutoFixFormatting.BraceStyle.otbs, "\t", 4, fileEol),
string file = __FILE__, size_t line = __LINE__)
{
import dparse.lexer : StringCache, Token;
import dscanner.analysis.run : parseModule;
import std.algorithm : canFind, findSplit, map, sort;
import std.conv : to;
import std.sumtype : match;
import std.typecons : tuple, Tuple;
StringCache cache = StringCache(StringCache.defaultBucketCount);
RollbackAllocator r;
const(Token)[] tokens;
const(Module) m = parseModule(file, cast(ubyte[]) before, &r, defaultErrorFormat, cache, false, tokens);
ModuleCache moduleCache;
// Run the code and get any warnings
MessageSet rawWarnings = analyze("test", m, config, moduleCache, tokens, true, true, formattingConfig);
string[] codeLines = before.splitLines();
Tuple!(Message, int)[] toApply;
int[] applyLines;
scope (failure)
{
if (toApply.length)
stderr.writefln("Would have applied these fixes:%(\n- %s%)",
toApply.map!"a[0].autofixes[a[1]].name");
else
stderr.writeln("Did not find any fixes at all up to this point.");
stderr.writeln("Found warnings on lines: ", rawWarnings[].map!(a
=> a.endLine == 0 ? 0 : a.endLine - 1 + line));
}
foreach (rawWarning; rawWarnings[])
{
// Skip the warning if it is on line zero
immutable size_t rawLine = rawWarning.endLine;
if (rawLine == 0)
{
stderr.writefln("!!! Skipping warning because it is on line zero:\n%s",
rawWarning.message);
continue;
}
auto fixComment = codeLines[rawLine - 1].findSplit("// fix");
if (fixComment[1].length)
{
applyLines ~= cast(int)rawLine - 1;
if (fixComment[2].startsWith(":"))
{
auto i = fixComment[2][1 .. $].to!int;
assert(i >= 0, "can't use negative autofix indices");
if (i >= rawWarning.autofixes.length)
throw new AssertError("autofix index out of range, diagnostic only has %s autofixes (%s)."
.format(rawWarning.autofixes.length, rawWarning.autofixes.map!"a.name"),
file, rawLine + line);
toApply ~= tuple(rawWarning, i);
}
else
{
if (rawWarning.autofixes.length != 1)
throw new AssertError("diagnostic has %s autofixes (%s), but expected exactly one."
.format(rawWarning.autofixes.length, rawWarning.autofixes.map!"a.name"),
file, rawLine + line);
toApply ~= tuple(rawWarning, 0);
}
}
}
foreach (i, codeLine; codeLines)
{
if (!applyLines.canFind(i) && codeLine.canFind("// fix"))
throw new AssertError("Missing expected warning for autofix on line %s"
.format(i + line), file, i + line);
}
AutoFix.CodeReplacement[] replacements;
foreach_reverse (pair; toApply)
{
Message message = pair[0];
AutoFix fix = message.autofixes[pair[1]];
replacements ~= fix.expectReplacements;
}
replacements.sort!"a.range[0] < b.range[0]";
improveAutoFixWhitespace(before, replacements);
string newCode = before;
foreach_reverse (replacement; replacements)
{
newCode = newCode[0 .. replacement.range[0]] ~ replacement.newText
~ newCode[replacement.range[1] .. $];
}
if (newCode != after)
{
bool onlyWhitespaceDiffers = newCode.replace("\t", "").replace(" ", "")
== after.replace("\t", "").replace(" ", "").replace("\r", "");
string formatDisplay(string code)
{
string ret = code.lineSplitter!(KeepTerminator.yes).map!(a => "\t" ~ a).join;
if (onlyWhitespaceDiffers)
ret = ret
.replace("\r", "\x1B[2m\\r\x1B[m")
.replace("\t", "\x1B[2m→ \x1B[m")
.replace(" ", "\x1B[2m⸱\x1B[m");
return ret;
}
throw new AssertError("Applying autofix didn't yield expected results. Expected:\n"
~ formatDisplay(after)
~ "\n\nActual:\n"
~ formatDisplay(newCode),
file, line);
}
}

View File

@ -20,9 +20,9 @@ final class IfConstraintsIndentCheck : BaseAnalyzer
mixin AnalyzerInfo!"if_constraints_indent";
///
this(string fileName, const(Token)[] tokens, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
// convert tokens to a list of token starting positions per line
@ -33,12 +33,12 @@ final class IfConstraintsIndentCheck : BaseAnalyzer
// t.line (unsigned) may be 0 if the token is uninitialized/broken, so don't subtract from it
// equivalent to: firstSymbolAtLine.length < t.line - 1
while (firstSymbolAtLine.length + 1 < t.line)
firstSymbolAtLine ~= Pos(1);
firstSymbolAtLine ~= Pos(1, t.index);
// insert a new line with positions if new line is reached
// (previous while pads skipped lines)
if (firstSymbolAtLine.length < t.line)
firstSymbolAtLine ~= Pos(t.column, t.type == tok!"if");
firstSymbolAtLine ~= Pos(t.column, t.index, t.type == tok!"if");
}
}
@ -96,6 +96,7 @@ private:
static struct Pos
{
size_t column;
size_t index;
bool isIf;
}
@ -109,17 +110,21 @@ private:
void checkConstraintSpace(const Constraint constraint, size_t line)
{
import std.algorithm : min;
// dscanner lines start at 1
auto pDecl = firstSymbolAtLine[line - 1];
// search for constraint if (might not be on the same line as the expression)
auto r = firstSymbolAtLine[line .. constraint.expression.line].retro.filter!(s => s.isIf);
auto if_ = constraint.tokens.findTokenForDisplay(tok!"if")[0];
// no hit = constraint is on the same line
if (r.empty)
addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE);
addErrorMessage(if_, KEY, MESSAGE);
else if (pDecl.column != r.front.column)
addErrorMessage(constraint.expression.line, constraint.expression.column, KEY, MESSAGE);
addErrorMessage([min(if_.index, pDecl.index), if_.index + 2], if_.line, [min(if_.column, pDecl.column), if_.column + 2], KEY, MESSAGE);
}
}
@ -138,8 +143,10 @@ void foo(R)(R r)
if (R == null)
{}
// note: since we are using tabs, the ^ look a bit off here. Use 1-wide tab stops to view tests.
void foo(R)(R r)
if (R == null) // [warn]: %s
if (R == null) /+
^^^ [warn]: %s +/
{}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
@ -151,11 +158,13 @@ void foo(R)(R r)
{}
void foo(R)(R r)
if (R == null) // [warn]: %s
if (R == null) /+
^^ [warn]: %s +/
{}
void foo(R)(R r)
if (R == null) // [warn]: %s
if (R == null) /+
^^^ [warn]: %s +/
{}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
@ -168,11 +177,13 @@ if (R == null) // [warn]: %s
{}
struct Foo(R)
if (R == null) // [warn]: %s
if (R == null) /+
^^ [warn]: %s +/
{}
struct Foo(R)
if (R == null) // [warn]: %s
if (R == null) /+
^^^ [warn]: %s +/
{}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
@ -206,8 +217,9 @@ if (is(typeof(Num.init >= 0)) && is(typeof(-Num.init)) &&
{}
struct Foo(R)
if
(R == null) // [warn]: %s
if /+
^^ [warn]: %s +/
(R == null)
{}
struct Foo(R)
@ -222,8 +234,9 @@ if
{}
struct Foo(R)
if (
R == null // [warn]: %s
if ( /+
^^^ [warn]: %s +/
R == null
) {}
}c.format(
IfConstraintsIndentCheck.MESSAGE,
@ -232,7 +245,8 @@ if
// constraint on the same line
assertAnalyzerWarnings(q{
struct CRC(uint N, ulong P) if (N == 32 || N == 64) // [warn]: %s
struct CRC(uint N, ulong P) if (N == 32 || N == 64) /+
^^ [warn]: %s +/
{}
}c.format(
IfConstraintsIndentCheck.MESSAGE,

View File

@ -9,15 +9,16 @@ import dparse.lexer;
import dparse.formatter;
import dscanner.analysis.base;
import dsymbol.scope_ : Scope;
import std.typecons : Rebindable, rebindable;
final class IfStatementCheck : BaseAnalyzer
{
alias visit = BaseAnalyzer.visit;
mixin AnalyzerInfo!"redundant_if_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const IfStatement ifStatement)
@ -28,14 +29,14 @@ final class IfStatementCheck : BaseAnalyzer
++depth;
if (ifStatement.expression.items.length == 1
&& (cast(AndAndExpression) ifStatement.expression.items[0]) is null)
if (ifStatement.condition.expression.items.length == 1
&& (cast(AndAndExpression) ifStatement.condition.expression.items[0]) is null)
{
redundancyCheck(ifStatement.expression,
ifStatement.expression.line, ifStatement.expression.column);
redundancyCheck(ifStatement.condition.expression,
ifStatement.condition.expression.line, ifStatement.condition.expression.column);
}
inIfExpresson = true;
ifStatement.expression.accept(this);
ifStatement.condition.expression.accept(this);
inIfExpresson = false;
ifStatement.thenStatement.accept(this);
if (expressions.length)
@ -81,12 +82,12 @@ private:
immutable size_t prevLocation = alreadyChecked(app.data, line, column);
if (prevLocation != size_t.max)
{
addErrorMessage(line, column, KEY, "Expression %s is true: already checked on line %d.".format(
addErrorMessage(expressions[prevLocation].astNode, KEY, "Expression %s is true: already checked on line %d.".format(
expressions[prevLocation].formatted, expressions[prevLocation].line));
}
else
{
expressions ~= ExpressionInfo(app.data, line, column, depth);
expressions ~= ExpressionInfo(app.data, line, column, depth, (cast(const BaseNode) expression).rebindable);
sort(expressions);
}
}
@ -124,4 +125,5 @@ private struct ExpressionInfo
size_t line;
size_t column;
int depth;
Rebindable!(const BaseNode) astNode;
}

View File

@ -26,16 +26,21 @@ final class IfElseSameCheck : BaseAnalyzer
mixin AnalyzerInfo!"if_else_same_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const IfStatement ifStatement)
{
if (ifStatement.thenStatement && (ifStatement.thenStatement == ifStatement.elseStatement))
addErrorMessage(ifStatement.line, ifStatement.column,
"dscanner.bugs.if_else_same", "'Else' branch is identical to 'Then' branch.");
{
const(Token)[] tokens = ifStatement.elseStatement.tokens;
// extend 1 past, so we include the `else` token
tokens = (tokens.ptr - 1)[0 .. tokens.length + 1];
addErrorMessage(tokens,
IF_ELSE_SAME_KEY, "'Else' branch is identical to 'Then' branch.");
}
ifStatement.accept(this);
}
@ -45,7 +50,7 @@ final class IfElseSameCheck : BaseAnalyzer
if (e !is null && assignExpression.operator == tok!"="
&& e.ternaryExpression == assignExpression.ternaryExpression)
{
addErrorMessage(assignExpression.line, assignExpression.column, "dscanner.bugs.self_assignment",
addErrorMessage(assignExpression, SELF_ASSIGNMENT_KEY,
"Left side of assignment operatior is identical to the right side.");
}
assignExpression.accept(this);
@ -56,8 +61,8 @@ final class IfElseSameCheck : BaseAnalyzer
if (andAndExpression.left !is null && andAndExpression.right !is null
&& andAndExpression.left == andAndExpression.right)
{
addErrorMessage(andAndExpression.line, andAndExpression.column,
"dscanner.bugs.logic_operator_operands",
addErrorMessage(andAndExpression.right,
LOGIC_OPERATOR_OPERANDS_KEY,
"Left side of logical and is identical to right side.");
}
andAndExpression.accept(this);
@ -68,12 +73,18 @@ final class IfElseSameCheck : BaseAnalyzer
if (orOrExpression.left !is null && orOrExpression.right !is null
&& orOrExpression.left == orOrExpression.right)
{
addErrorMessage(orOrExpression.line, orOrExpression.column,
"dscanner.bugs.logic_operator_operands",
addErrorMessage(orOrExpression.right,
LOGIC_OPERATOR_OPERANDS_KEY,
"Left side of logical or is identical to right side.");
}
orOrExpression.accept(this);
}
private:
enum string IF_ELSE_SAME_KEY = "dscanner.bugs.if_else_same";
enum string SELF_ASSIGNMENT_KEY = "dscanner.bugs.self_assignment";
enum string LOGIC_OPERATOR_OPERANDS_KEY = "dscanner.bugs.logic_operator_operands";
}
unittest
@ -86,10 +97,13 @@ unittest
void testSizeT()
{
string person = "unknown";
if (person == "unknown") // [warn]: 'Else' branch is identical to 'Then' branch.
person = "bobrick"; // same
if (person == "unknown")
person = "bobrick"; /* same */
else
person = "bobrick"; // same
person = "bobrick"; /* same */ /+
^^^^^^^^^^^^^^^^^^^^^^^ [warn]: 'Else' branch is identical to 'Then' branch. +/
// note: above ^^^ line spans over multiple lines, so it starts at start of line, since we don't have any way to test this here
// look at the tests using 1-wide tab width for accurate visuals.
if (person == "unknown") // ok
person = "ricky"; // not same

View File

@ -20,9 +20,9 @@ final class ImportSortednessCheck : BaseAnalyzer
mixin AnalyzerInfo!"imports_sortedness";
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
mixin ScopedVisit!Module;
@ -46,19 +46,21 @@ final class ImportSortednessCheck : BaseAnalyzer
if (id.importBindings is null || id.importBindings.importBinds.length == 0)
{
bool suppress;
foreach (singleImport; id.singleImports)
{
string importModuleName = singleImport.identifierChain.identifiers.map!`a.text`.join(".");
addImport(importModuleName, singleImport);
addImport(importModuleName, singleImport, null, suppress);
}
}
else
{
string importModuleName = id.importBindings.singleImport.identifierChain.identifiers.map!`a.text`.join(".");
bool suppress;
foreach (importBind; id.importBindings.importBinds)
{
addImport(importModuleName ~ "-" ~ importBind.left.text, id.importBindings.singleImport);
addImport(importModuleName ~ "-" ~ importBind.left.text, importBind, id.importBindings.singleImport, suppress);
}
}
}
@ -80,14 +82,29 @@ private:
}
}
void addImport(string importModuleName, const SingleImport singleImport)
void addImport(string importModuleName, const BaseNode range, const BaseNode parent, ref bool suppress)
{
import std.algorithm : findSplit;
import std.string : indexOf;
import std.uni : sicmp;
if (imports[level].length > 0 && imports[level][$ -1].sicmp(importModuleName) > 0)
{
addErrorMessage(singleImport.identifierChain.identifiers[0].line,
singleImport.identifierChain.identifiers[0].column, KEY, MESSAGE);
if (parent !is null)
{
auto parentEnd = importModuleName.indexOf("-");
if (parentEnd != -1 && imports[level][$ -1].findSplit("-")[0].sicmp(importModuleName) > 0)
{
// mark module name as broken, not selected symbols, since it's the module name is not belonging here
if (!suppress)
addErrorMessage(parent, KEY, MESSAGE);
suppress = true;
return;
}
}
if (!suppress)
addErrorMessage(range, KEY, MESSAGE);
suppress = true;
}
else
{
@ -113,7 +130,8 @@ unittest
assertAnalyzerWarnings(q{
import foo.bar;
import bar.foo; // [warn]: %s
import bar.foo; /+
^^^^^^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
), sac);
@ -121,19 +139,36 @@ unittest
assertAnalyzerWarnings(q{
import c;
import c.b;
import c.a; // [warn]: %s
import c.a; /+
^^^ [warn]: %s +/
import d.a;
import d; // [warn]: %s
import d; /+
^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
), sac);
assertAnalyzerWarnings(q{
import a.b, a.c, a.d;
import a.b, a.d, a.c; // [warn]: %s
import a.c, a.b, a.c; // [warn]: %s
import foo.bar, bar.foo; // [warn]: %s
unittest
{
import a.b, a.c, a.d;
}
unittest
{
import a.b, a.d, a.c; /+
^^^ [warn]: %s +/
}
unittest
{
import a.c, a.b, a.c; /+
^^^ [warn]: %s +/
}
unittest
{
import foo.bar, bar.foo; /+
^^^^^^^ [warn]: %s +/
}
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -143,8 +178,10 @@ unittest
// multiple items out of order
assertAnalyzerWarnings(q{
import foo.bar;
import bar.foo; // [warn]: %s
import bar.bar.foo; // [warn]: %s
import bar.foo; /+
^^^^^^^ [warn]: %s +/
import bar.bar.foo; /+
^^^^^^^^^^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -158,14 +195,19 @@ unittest
// selective imports
assertAnalyzerWarnings(q{
import test : foo;
import test : bar; // [warn]: %s
import test : bar; /+
^^^ [warn]: %s +/
import before : zzz; /+
^^^^^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
), sac);
// selective imports
assertAnalyzerWarnings(q{
import test : foo, bar; // [warn]: %s
import test : foo, bar; /+
^^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
), sac);
@ -173,8 +215,10 @@ unittest
assertAnalyzerWarnings(q{
import b;
import c : foo;
import c : bar; // [warn]: %s
import a; // [warn]: %s
import c : bar; /+
^^^ [warn]: %s +/
import a; /+
^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -184,8 +228,10 @@ unittest
import c;
import c : bar;
import d : bar;
import d; // [warn]: %s
import a : bar; // [warn]: %s
import d; /+
^ [warn]: %s +/
import a : bar; /+
^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -199,8 +245,10 @@ unittest
assertAnalyzerWarnings(q{
import t1 : a, b = foo;
import t1 : b, a = foo; // [warn]: %s
import t0 : a, b = foo; // [warn]: %s
import t1 : b, a = foo; /+
^^^^^^^ [warn]: %s +/
import t0 : a, b = foo; /+
^^ [warn]: %s +/
}c.format(
ImportSortednessCheck.MESSAGE,
ImportSortednessCheck.MESSAGE,
@ -209,11 +257,13 @@ unittest
// local imports in functions
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
void foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
void bar()
@ -229,15 +279,18 @@ unittest
// local imports in scopes
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
void foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
{
@ -255,15 +308,18 @@ unittest
// local imports in functions
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
void foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
while (true) {
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
for (;;) {
@ -273,7 +329,8 @@ unittest
}
foreach (el; arr) {
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
}
@ -287,23 +344,28 @@ unittest
// nested scopes
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
void foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
}
@ -320,11 +382,13 @@ unittest
// local imports in functions
assertAnalyzerWarnings(q{
import t2;
import t1; // [warn]: %s
import t1; /+
^^ [warn]: %s +/
struct foo()
{
import f2;
import f1; // [warn]: %s
import f1; /+
^^ [warn]: %s +/
import f3;
}
class bar()

View File

@ -10,6 +10,8 @@ import dscanner.analysis.helpers;
import dparse.ast;
import dparse.lexer;
import std.typecons : Rebindable;
/**
* Checks for incorrect infinite range definitions
*/
@ -20,9 +22,9 @@ final class IncorrectInfiniteRangeCheck : BaseAnalyzer
mixin AnalyzerInfo!"incorrect_infinite_range_check";
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const StructBody structBody)
@ -36,9 +38,10 @@ final class IncorrectInfiniteRangeCheck : BaseAnalyzer
{
if (inStruct > 0 && fd.name.text == "empty")
{
line = fd.name.line;
column = fd.name.column;
auto old = parentFunc;
parentFunc = fd;
fd.accept(this);
parentFunc = old;
}
}
@ -63,7 +66,7 @@ final class IncorrectInfiniteRangeCheck : BaseAnalyzer
override void visit(const ReturnStatement rs)
{
if (inStruct == 0 || line == size_t.max) // not within a struct yet
if (inStruct == 0 || parentFunc == null) // not within a struct yet
return;
visitReturnExpression(rs.expression);
}
@ -79,7 +82,7 @@ final class IncorrectInfiniteRangeCheck : BaseAnalyzer
return;
if (unary.primaryExpression.primary != tok!"false")
return;
addErrorMessage(line, column, KEY, MESSAGE);
addErrorMessage(parentFunc.get, KEY, MESSAGE);
}
override void visit(const Unittest u)
@ -90,8 +93,7 @@ private:
uint inStruct;
enum string KEY = "dscanner.suspicious.incorrect_infinite_range";
enum string MESSAGE = "Use `enum bool empty = false;` to define an infinite range.";
size_t line = size_t.max;
size_t column;
Rebindable!(const FunctionDeclaration) parentFunc;
}
unittest
@ -104,10 +106,12 @@ unittest
sac.incorrect_infinite_range_check = Check.enabled;
assertAnalyzerWarnings(q{struct InfiniteRange
{
bool empty() // [warn]: %1$s
bool empty()
{
return false;
}
} /+
^^ [warn]: %1$s+/
// TODO: test for multiline issues like this
bool stuff()
{
@ -128,7 +132,8 @@ unittest
struct InfiniteRange
{
bool empty() => false; // [warn]: %1$s
bool empty() => false; /+
^^^^^^^^^^^^^^^^^^^^^^ [warn]: %1$s +/
bool stuff() => false;
unittest
{
@ -143,7 +148,8 @@ struct InfiniteRange
}
bool empty() { return false; }
class C { bool empty() { return false; } } // [warn]: %1$s
class C { bool empty() { return false; } } /+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ [warn]: %1$s +/
}c
.format(IncorrectInfiniteRangeCheck.MESSAGE), sac);

View File

@ -13,23 +13,15 @@ import dscanner.analysis.helpers;
/**
* Checks for labels and variables that have the same name.
*/
final class LabelVarNameCheck : BaseAnalyzer
final class LabelVarNameCheck : ScopedBaseAnalyzer
{
mixin AnalyzerInfo!"label_var_same_name_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
mixin ScopedVisit!Module;
mixin ScopedVisit!BlockStatement;
mixin ScopedVisit!StructBody;
mixin ScopedVisit!CaseStatement;
mixin ScopedVisit!ForStatement;
mixin ScopedVisit!IfStatement;
mixin ScopedVisit!TemplateDeclaration;
mixin AggregateVisit!ClassDeclaration;
mixin AggregateVisit!StructDeclaration;
mixin AggregateVisit!InterfaceDeclaration;
@ -64,10 +56,12 @@ final class LabelVarNameCheck : BaseAnalyzer
--conditionalDepth;
}
alias visit = BaseAnalyzer.visit;
alias visit = ScopedBaseAnalyzer.visit;
private:
enum string KEY = "dscanner.suspicious.label_var_same_name";
Thing[string][] stack;
template AggregateVisit(NodeType)
@ -80,16 +74,6 @@ private:
}
}
template ScopedVisit(NodeType)
{
override void visit(const NodeType n)
{
pushScope();
n.accept(this);
popScope();
}
}
void duplicateCheck(const Token name, bool fromLabel, bool isConditional)
{
import std.conv : to;
@ -106,7 +90,7 @@ private:
{
immutable thisKind = fromLabel ? "Label" : "Variable";
immutable otherKind = thing.isVar ? "variable" : "label";
addErrorMessage(name.line, name.column, "dscanner.suspicious.label_var_same_name",
addErrorMessage(name, KEY,
thisKind ~ " \"" ~ fqn ~ "\" has the same name as a "
~ otherKind ~ " defined on line " ~ to!string(thing.line) ~ ".");
}
@ -128,12 +112,12 @@ private:
return stack[$ - 1];
}
void pushScope()
protected override void pushScope()
{
stack.length++;
}
void popScope()
protected override void popScope()
{
stack.length--;
}
@ -178,14 +162,16 @@ unittest
unittest
{
blah:
int blah; // [warn]: Variable "blah" has the same name as a label defined on line 4.
int blah; /+
^^^^ [warn]: Variable "blah" has the same name as a label defined on line 4. +/
}
int blah;
unittest
{
static if (stuff)
int a;
int a; // [warn]: Variable "a" has the same name as a variable defined on line 11.
int a; /+
^ [warn]: Variable "a" has the same name as a variable defined on line 12. +/
}
unittest
@ -202,7 +188,8 @@ unittest
int a = 10;
else
int a = 20;
int a; // [warn]: Variable "a" has the same name as a variable defined on line 28.
int a; /+
^ [warn]: Variable "a" has the same name as a variable defined on line 30. +/
}
template T(stuff)
{
@ -225,7 +212,8 @@ unittest
int c = 10;
else
int c = 20;
int c; // [warn]: Variable "c" has the same name as a variable defined on line 51.
int c; /+
^ [warn]: Variable "c" has the same name as a variable defined on line 54. +/
}
unittest
@ -263,7 +251,8 @@ unittest
interface A
{
int a;
int a; // [warn]: Variable "A.a" has the same name as a variable defined on line 89.
int a; /+
^ [warn]: Variable "A.a" has the same name as a variable defined on line 93. +/
}
}
@ -273,6 +262,21 @@ unittest
struct a { int a; }
}
unittest
{
switch (1) {
case 1:
int x, c1;
break;
case 2:
int x, c2;
break;
default:
int x, def;
break;
}
}
}c, sac);
stderr.writeln("Unittest for LabelVarNameCheck passed.");
}

View File

@ -16,13 +16,15 @@ final class LambdaReturnCheck : BaseAnalyzer
mixin AnalyzerInfo!"lambda_return_check";
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const FunctionLiteralExpression fLit)
{
import std.algorithm : find;
auto fe = safeAccess(fLit).assignExpression.as!UnaryExpression
.primaryExpression.functionLiteralExpression.unwrap;
@ -31,7 +33,25 @@ final class LambdaReturnCheck : BaseAnalyzer
{
return;
}
addErrorMessage(fLit.line, fLit.column, KEY, "This lambda returns a lambda. Add parenthesis to clarify.");
auto start = &fLit.tokens[0];
auto endIncl = &fe.specifiedFunctionBody.tokens[0];
assert(endIncl >= start);
auto tokens = start[0 .. endIncl - start + 1];
auto arrow = tokens.find!(a => a.type == tok!"=>");
AutoFix[] autofixes;
if (arrow.length)
{
if (fLit.tokens[0] == tok!"(")
autofixes ~= AutoFix.replacement(arrow[0], "", "Remove arrow (use function body)");
else
autofixes ~= AutoFix.insertionBefore(fLit.tokens[0], "(", "Remove arrow (use function body)")
.concat(AutoFix.insertionAfter(fLit.tokens[0], ")"))
.concat(AutoFix.replacement(arrow[0], ""));
}
autofixes ~= AutoFix.insertionBefore(*endIncl, "() ", "Add parenthesis (return delegate)");
addErrorMessage(tokens, KEY, "This lambda returns a lambda. Add parenthesis to clarify.",
autofixes);
}
private:
@ -41,23 +61,52 @@ private:
version(Windows) {/*because of newline in code*/} else
unittest
{
import dscanner.analysis.helpers : assertAnalyzerWarnings;
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig();
sac.lambda_return_check = Check.enabled;
auto code = `
assertAnalyzerWarnings(q{
void main()
{
int[] b;
auto a = b.map!(a => { return a * a + 2; }).array(); // [warn]: This lambda returns a lambda. Add parenthesis to clarify.
pragma(msg, typeof(a => { return a; })); // [warn]: This lambda returns a lambda. Add parenthesis to clarify.
pragma(msg, typeof((a) => { return a; })); // [warn]: This lambda returns a lambda. Add parenthesis to clarify.
auto a = b.map!(a => { return a * a + 2; }).array(); /+
^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/
pragma(msg, typeof(a => { return a; })); /+
^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/
pragma(msg, typeof((a) => { return a; })); /+
^^^^^^^^ [warn]: This lambda returns a lambda. Add parenthesis to clarify. +/
pragma(msg, typeof({ return a; }));
pragma(msg, typeof(a => () { return a; }));
}`c;
assertAnalyzerWarnings(code, sac);
}
}c, sac);
assertAutoFix(q{
void main()
{
int[] b;
auto a = b.map!(a => { return a * a + 2; }).array(); // fix:0
auto a = b.map!(a => { return a * a + 2; }).array(); // fix:1
pragma(msg, typeof(a => { return a; })); // fix:0
pragma(msg, typeof(a => { return a; })); // fix:1
pragma(msg, typeof((a) => { return a; })); // fix:0
pragma(msg, typeof((a) => { return a; })); // fix:1
}
}c, q{
void main()
{
int[] b;
auto a = b.map!((a) { return a * a + 2; }).array(); // fix:0
auto a = b.map!(a => () { return a * a + 2; }).array(); // fix:1
pragma(msg, typeof((a) { return a; })); // fix:0
pragma(msg, typeof(a => () { return a; })); // fix:1
pragma(msg, typeof((a) { return a; })); // fix:0
pragma(msg, typeof((a) => () { return a; })); // fix:1
}
}c, sac);
stderr.writeln("Unittest for LambdaReturnCheck passed.");
}

View File

@ -18,13 +18,15 @@ import dsymbol.scope_;
*/
final class LengthSubtractionCheck : BaseAnalyzer
{
private enum string KEY = "dscanner.suspicious.length_subtraction";
alias visit = BaseAnalyzer.visit;
mixin AnalyzerInfo!"length_subtraction_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const AddExpression addExpression)
@ -34,24 +36,17 @@ final class LengthSubtractionCheck : BaseAnalyzer
const UnaryExpression l = cast(const UnaryExpression) addExpression.left;
const UnaryExpression r = cast(const UnaryExpression) addExpression.right;
if (l is null || r is null)
{
// stderr.writeln(__FILE__, " ", __LINE__);
goto end;
}
if (r.primaryExpression is null || r.primaryExpression.primary.type != tok!"intLiteral")
{
// stderr.writeln(__FILE__, " ", __LINE__);
goto end;
}
if (l.identifierOrTemplateInstance is null
|| l.identifierOrTemplateInstance.identifier.text != "length")
{
// stderr.writeln(__FILE__, " ", __LINE__);
goto end;
}
const(Token) token = l.identifierOrTemplateInstance.identifier;
addErrorMessage(token.line, token.column, "dscanner.suspicious.length_subtraction",
"Avoid subtracting from '.length' as it may be unsigned.");
addErrorMessage(addExpression, KEY,
"Avoid subtracting from '.length' as it may be unsigned.",
[
AutoFix.insertionBefore(l.tokens[0], "cast(ptrdiff_t) ", "Cast to ptrdiff_t")
]);
}
end:
addExpression.accept(this);
@ -67,7 +62,22 @@ unittest
assertAnalyzerWarnings(q{
void testSizeT()
{
if (i < a.length - 1) // [warn]: Avoid subtracting from '.length' as it may be unsigned.
if (i < a.length - 1) /+
^^^^^^^^^^^^ [warn]: Avoid subtracting from '.length' as it may be unsigned. +/
writeln("something");
}
}c, sac);
assertAutoFix(q{
void testSizeT()
{
if (i < a.length - 1) // fix
writeln("something");
}
}c, q{
void testSizeT()
{
if (i < cast(ptrdiff_t) a.length - 1) // fix
writeln("something");
}
}c, sac);

View File

@ -20,10 +20,9 @@ final class LineLengthCheck : BaseAnalyzer
mixin AnalyzerInfo!"long_line_check";
///
this(string fileName, const(Token)[] tokens, int maxLineLength, bool skipTests = false)
this(BaseAnalyzerArguments args, int maxLineLength)
{
super(fileName, null, skipTests);
this.tokens = tokens;
super(args);
this.maxLineLength = maxLineLength;
}
@ -58,9 +57,11 @@ private:
void triggerError(ref const Token tok)
{
import std.algorithm : max;
if (tok.line != lastErrorLine)
{
addErrorMessage(tok.line, tok.column, KEY, message);
addErrorMessage([0, 0], tok.line, [maxLineLength, max(maxLineLength + 1, tok.column + 1)], KEY, message);
lastErrorLine = tok.line;
}
}
@ -92,9 +93,9 @@ private:
unittest
{
assert(new LineLengthCheck(null, null, 120).checkMultiLineToken(Token(tok!"stringLiteral", " ", 0, 0, 0)) == 8);
assert(new LineLengthCheck(null, null, 120).checkMultiLineToken(Token(tok!"stringLiteral", " \na", 0, 0, 0)) == 2);
assert(new LineLengthCheck(null, null, 120).checkMultiLineToken(Token(tok!"stringLiteral", " \n ", 0, 0, 0)) == 5);
assert(new LineLengthCheck(BaseAnalyzerArguments.init, 120).checkMultiLineToken(Token(tok!"stringLiteral", " ", 0, 0, 0)) == 8);
assert(new LineLengthCheck(BaseAnalyzerArguments.init, 120).checkMultiLineToken(Token(tok!"stringLiteral", " \na", 0, 0, 0)) == 2);
assert(new LineLengthCheck(BaseAnalyzerArguments.init, 120).checkMultiLineToken(Token(tok!"stringLiteral", " \n ", 0, 0, 0)) == 5);
}
static size_t tokenByteLength()(auto ref const Token tok)
@ -163,7 +164,6 @@ private:
enum string KEY = "dscanner.style.long_line";
const int maxLineLength;
const(Token)[] tokens;
}
@system unittest

View File

@ -25,9 +25,9 @@ final class LocalImportCheck : BaseAnalyzer
/**
* Construct with the given file name.
*/
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
mixin visitThing!StructBody;
@ -58,9 +58,8 @@ final class LocalImportCheck : BaseAnalyzer
{
if (singleImport.rename.text.length == 0)
{
addErrorMessage(singleImport.identifierChain.identifiers[0].line,
singleImport.identifierChain.identifiers[0].column,
"dscanner.suspicious.local_imports", "Local imports should specify"
addErrorMessage(singleImport,
KEY, "Local imports should specify"
~ " the symbols being imported to avoid hiding local symbols.");
}
}
@ -69,6 +68,8 @@ final class LocalImportCheck : BaseAnalyzer
private:
enum string KEY = "dscanner.suspicious.local_imports";
mixin template visitThing(T)
{
override void visit(const T thing)
@ -93,7 +94,8 @@ unittest
assertAnalyzerWarnings(q{
void testLocalImport()
{
import std.stdio; // [warn]: Local imports should specify the symbols being imported to avoid hiding local symbols.
import std.stdio; /+
^^^^^^^^^ [warn]: Local imports should specify the symbols being imported to avoid hiding local symbols. +/
import std.fish : scales, head;
import DAGRON = std.experimental.dragon;
}

View File

@ -26,9 +26,9 @@ final class LogicPrecedenceCheck : BaseAnalyzer
enum string KEY = "dscanner.confusing.logical_precedence";
mixin AnalyzerInfo!"logical_precedence_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const OrOrExpression orOr)
@ -41,7 +41,7 @@ final class LogicPrecedenceCheck : BaseAnalyzer
return;
if ((left !is null && left.right is null) && (right !is null && right.right is null))
return;
addErrorMessage(orOr.line, orOr.column, KEY,
addErrorMessage(orOr, KEY,
"Use parenthesis to clarify this expression.");
orOr.accept(this);
}
@ -56,9 +56,11 @@ unittest
assertAnalyzerWarnings(q{
void testFish()
{
if (a && b || c) {} // [warn]: Use parenthesis to clarify this expression.
if (a && b || c) {} /+
^^^^^^^^^^^ [warn]: Use parenthesis to clarify this expression. +/
if ((a && b) || c) {} // Good
if (b || c && d) {} // [warn]: Use parenthesis to clarify this expression.
if (b || c && d) {} /+
^^^^^^^^^^^ [warn]: Use parenthesis to clarify this expression. +/
if (b || (c && d)) {} // Good
}
}c, sac);

View File

@ -5,7 +5,7 @@ import dscanner.utils : safeAccess;
import dsymbol.scope_;
import dsymbol.symbol;
import dparse.ast;
import dparse.lexer : tok;
import dparse.lexer : tok, Token;
import dsymbol.builtin.names;
/// Checks for mismatched argument and parameter names
@ -14,9 +14,9 @@ final class MismatchedArgumentCheck : BaseAnalyzer
mixin AnalyzerInfo!"mismatched_args_check";
///
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const FunctionCallExpression fce)
@ -42,8 +42,7 @@ final class MismatchedArgumentCheck : BaseAnalyzer
static struct ErrorMessage
{
size_t line;
size_t column;
const(Token)[] range;
string message;
}
@ -60,17 +59,16 @@ final class MismatchedArgumentCheck : BaseAnalyzer
matched = true;
else
{
foreach (size_t i, ref const mm; mismatches)
foreach (ref const mm; mismatches)
{
messages ~= ErrorMessage(argVisitor.lines[i],
argVisitor.columns[i], createWarningFromMismatch(mm));
messages ~= ErrorMessage(argVisitor.tokens[mm.argIndex], createWarningFromMismatch(mm));
}
}
}
if (!matched)
foreach (m; messages)
addErrorMessage(m.line, m.column, KEY, m.message);
addErrorMessage(m.range, KEY, m.message);
}
alias visit = ASTVisitor.visit;
@ -109,18 +107,21 @@ final class IdentVisitor : ASTVisitor
final class ArgVisitor : ASTVisitor
{
override void visit(const ArgumentList al)
override void visit(const NamedArgumentList al)
{
foreach (a; al.items)
{
auto u = cast(UnaryExpression) a;
if (u !is null)
auto u = cast(UnaryExpression) a.assignExpression;
size_t prevArgs = args.length;
if (u !is null && !a.name.text.length)
visit(u);
else
if (args.length == prevArgs)
{
// if we didn't get an identifier in the unary expression,
// assume it's a good argument
args ~= istring.init;
lines ~= size_t.max;
columns ~= size_t.max;
tokens ~= a.tokens;
}
}
}
@ -134,16 +135,14 @@ final class ArgVisitor : ASTVisitor
if (iot.identifier == tok!"")
return;
immutable t = iot.identifier;
lines ~= t.line;
columns ~= t.column;
tokens ~= [t];
args ~= internString(t.text);
}
}
alias visit = ASTVisitor.visit;
size_t[] lines;
size_t[] columns;
const(Token[])[] tokens;
istring[] args;
}
@ -249,3 +248,38 @@ unittest
assert(res == []);
}
}
unittest
{
import dscanner.analysis.helpers : assertAnalyzerWarnings;
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig();
sac.mismatched_args_check = Check.enabled;
assertAnalyzerWarnings(q{
void foo(int x, int y)
{
}
void bar()
{
int x = 1;
int y = 2;
foo(y, x); /+
^ [warn]: Argument 2 is named 'x', but this is the name of parameter 1 +/
foo(y + 1, x); /+
^ [warn]: Argument 2 is named 'x', but this is the name of parameter 1 +/
foo(y + 1, f(x));
foo(x: y, y: x);
foo(y, 0); /+
^ [warn]: Argument 1 is named 'y', but this is the name of parameter 2 +/
// foo(y: y, x); // TODO: this shouldn't error
foo(x, y: x); // TODO: this should error
foo(y, y: x); /+
^ [warn]: Argument 1 is named 'y', but this is the name of parameter 2 +/
}
}c, sac);
stderr.writeln("Unittest for MismatchedArgumentCheck passed.");
}

View File

@ -0,0 +1,271 @@
module dscanner.analysis.nolint;
@safe:
import dparse.ast;
import dparse.lexer;
import std.algorithm : canFind;
import std.regex : matchAll, regex;
import std.string : lastIndexOf, strip;
import std.typecons;
struct NoLint
{
bool containsCheck(scope const(char)[] check) const
{
while (true)
{
if (disabledChecks.get((() @trusted => cast(string) check)(), 0) > 0)
return true;
auto dot = check.lastIndexOf('.');
if (dot == -1)
break;
check = check[0 .. dot];
}
return false;
}
// automatic pop when returned value goes out of scope
Poppable push(in Nullable!NoLint other) scope
{
if (other.isNull)
return Poppable(null);
foreach (key, value; other.get.getDisabledChecks)
this.disabledChecks[key] += value;
return Poppable(() => this.pop(other));
}
package:
const(int[string]) getDisabledChecks() const
{
return this.disabledChecks;
}
void pushCheck(in string check)
{
disabledChecks[check]++;
}
void merge(in Nullable!NoLint other)
{
if (other.isNull)
return;
foreach (key, value; other.get.getDisabledChecks)
this.disabledChecks[key] += value;
}
private:
void pop(in Nullable!NoLint other)
{
if (other.isNull)
return;
foreach (key, value; other.get.getDisabledChecks)
{
assert(this.disabledChecks.get(key, 0) >= value);
this.disabledChecks[key] -= value;
}
}
static struct Poppable
{
~this()
{
if (onPop)
onPop();
onPop = null;
}
private:
void delegate() onPop;
}
int[string] disabledChecks;
}
struct NoLintFactory
{
static Nullable!NoLint fromModuleDeclaration(in ModuleDeclaration moduleDeclaration)
{
NoLint noLint;
foreach (atAttribute; moduleDeclaration.atAttributes)
noLint.merge(NoLintFactory.fromAtAttribute(atAttribute));
if (!noLint.getDisabledChecks.length)
return nullNoLint;
return noLint.nullable;
}
static Nullable!NoLint fromDeclaration(in Declaration declaration)
{
NoLint noLint;
foreach (attribute; declaration.attributes)
noLint.merge(NoLintFactory.fromAttribute(attribute));
if (!noLint.getDisabledChecks.length)
return nullNoLint;
return noLint.nullable;
}
private:
static Nullable!NoLint fromAttribute(const(Attribute) attribute)
{
if (attribute is null)
return nullNoLint;
return NoLintFactory.fromAtAttribute(attribute.atAttribute);
}
static Nullable!NoLint fromAtAttribute(const(AtAttribute) atAttribute)
{
if (atAttribute is null)
return nullNoLint;
auto ident = atAttribute.identifier;
auto argumentList = atAttribute.argumentList;
if (argumentList !is null)
{
if (ident.text.length)
return NoLintFactory.fromStructUda(ident, argumentList);
else
return NoLintFactory.fromStringUda(argumentList);
}
else
return nullNoLint;
}
// @nolint("..")
static Nullable!NoLint fromStructUda(in Token ident, in ArgumentList argumentList)
in (ident.text.length && argumentList !is null)
{
if (ident.text != "nolint")
return nullNoLint;
NoLint noLint;
foreach (nodeExpr; argumentList.items)
{
if (auto unaryExpr = cast(const UnaryExpression) nodeExpr)
{
auto primaryExpression = unaryExpr.primaryExpression;
if (primaryExpression is null)
continue;
if (primaryExpression.primary != tok!"stringLiteral")
continue;
noLint.pushCheck(primaryExpression.primary.text.strip("\""));
}
}
if (!noLint.getDisabledChecks().length)
return nullNoLint;
return noLint.nullable;
}
// @("nolint(..)")
static Nullable!NoLint fromStringUda(in ArgumentList argumentList)
in (argumentList !is null)
{
NoLint noLint;
foreach (nodeExpr; argumentList.items)
{
if (auto unaryExpr = cast(const UnaryExpression) nodeExpr)
{
auto primaryExpression = unaryExpr.primaryExpression;
if (primaryExpression is null)
continue;
if (primaryExpression.primary != tok!"stringLiteral")
continue;
auto str = primaryExpression.primary.text.strip("\"");
Nullable!NoLint currNoLint = NoLintFactory.fromString(str);
noLint.merge(currNoLint);
}
}
if (!noLint.getDisabledChecks().length)
return nullNoLint;
return noLint.nullable;
}
// Transform a string with form "nolint(abc, efg)"
// into a NoLint struct
static Nullable!NoLint fromString(in string str)
{
static immutable re = regex(`[\w-_.]+`, "g");
auto matches = matchAll(str, re);
if (!matches)
return nullNoLint;
const udaName = matches.hit;
if (udaName != "nolint")
return nullNoLint;
matches.popFront;
NoLint noLint;
while (matches)
{
noLint.pushCheck(matches.hit);
matches.popFront;
}
if (!noLint.getDisabledChecks.length)
return nullNoLint;
return noLint.nullable;
}
static nullNoLint = Nullable!NoLint.init;
}
unittest
{
const s1 = "nolint(abc)";
const s2 = "nolint(abc, efg, hij)";
const s3 = " nolint ( abc , efg ) ";
const s4 = "nolint(dscanner.style.abc_efg-ijh)";
const s5 = "OtherUda(abc)";
const s6 = "nolint(dscanner)";
assert(NoLintFactory.fromString(s1).get.containsCheck("abc"));
assert(NoLintFactory.fromString(s2).get.containsCheck("abc"));
assert(NoLintFactory.fromString(s2).get.containsCheck("efg"));
assert(NoLintFactory.fromString(s2).get.containsCheck("hij"));
assert(NoLintFactory.fromString(s3).get.containsCheck("abc"));
assert(NoLintFactory.fromString(s3).get.containsCheck("efg"));
assert(NoLintFactory.fromString(s4).get.containsCheck("dscanner.style.abc_efg-ijh"));
assert(NoLintFactory.fromString(s5).isNull);
assert(NoLintFactory.fromString(s6).get.containsCheck("dscanner"));
assert(!NoLintFactory.fromString(s6).get.containsCheck("dscanner2"));
assert(NoLintFactory.fromString(s6).get.containsCheck("dscanner.foo"));
import std.stdio : stderr, writeln;
(() @trusted => stderr.writeln("Unittest for NoLint passed."))();
}

View File

@ -26,9 +26,9 @@ public:
/**
* Constructs the style checker with the given file name.
*/
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const Token t)
@ -39,12 +39,15 @@ public:
&& ((t.text.startsWith("0b") && !t.text.matchFirst(badBinaryRegex)
.empty) || !t.text.matchFirst(badDecimalRegex).empty))
{
addErrorMessage(t.line, t.column, "dscanner.style.number_literals",
addErrorMessage(t, KEY,
"Use underscores to improve number constant readability.");
}
}
private:
enum string KEY = "dscanner.style.number_literals";
auto badBinaryRegex = ctRegex!(`^0b[01]{9,}`);
auto badDecimalRegex = ctRegex!(`^\d{5,}`);
}
@ -62,10 +65,13 @@ unittest
a = 1; // ok
a = 10; // ok
a = 100; // ok
a = 1000; // FIXME: boom
a = 10000; // [warn]: Use underscores to improve number constant readability.
a = 100000; // [warn]: Use underscores to improve number constant readability.
a = 1000000; // [warn]: Use underscores to improve number constant readability.
a = 1000; // ok
a = 10000; /+
^^^^^ [warn]: Use underscores to improve number constant readability. +/
a = 100000; /+
^^^^^^ [warn]: Use underscores to improve number constant readability. +/
a = 1000000; /+
^^^^^^^ [warn]: Use underscores to improve number constant readability. +/
}
}c, sac);

View File

@ -24,9 +24,9 @@ final class ObjectConstCheck : BaseAnalyzer
mixin AnalyzerInfo!"object_const_check";
///
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
mixin visitTemplate!ClassDeclaration;
@ -68,8 +68,7 @@ final class ObjectConstCheck : BaseAnalyzer
if (inAggregate && !constColon && !constBlock && !isDeclationDisabled
&& isInteresting(fd.name.text) && !hasConst(fd.memberFunctionAttributes))
{
addErrorMessage(d.functionDeclaration.name.line,
d.functionDeclaration.name.column, "dscanner.suspicious.object_const",
addErrorMessage(d.functionDeclaration.name, KEY,
"Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.");
}
}
@ -82,7 +81,11 @@ final class ObjectConstCheck : BaseAnalyzer
constBlock = false;
}
private static bool hasConst(const MemberFunctionAttribute[] attributes)
private:
enum string KEY = "dscanner.suspicious.object_const";
static bool hasConst(const MemberFunctionAttribute[] attributes)
{
import std.algorithm : any;
@ -90,15 +93,14 @@ final class ObjectConstCheck : BaseAnalyzer
|| a.tokenType == tok!"immutable" || a.tokenType == tok!"inout");
}
private static bool isInteresting(string name)
static bool isInteresting(string name)
{
return name == "opCmp" || name == "toHash" || name == "opEquals"
|| name == "toString" || name == "opCast";
}
private bool constBlock;
private bool constColon;
bool constBlock;
bool constColon;
}
unittest
@ -157,22 +159,26 @@ unittest
// Will warn, because none are const
class Dog
{
bool opEquals(Object a, Object b) // [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.
bool opEquals(Object a, Object b) /+
^^^^^^^^ [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const. +/
{
return true;
}
int opCmp(Object o) // [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.
int opCmp(Object o) /+
^^^^^ [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const. +/
{
return 1;
}
hash_t toHash() // [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.
hash_t toHash() /+
^^^^^^ [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const. +/
{
return 0;
}
string toString() // [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const.
string toString() /+
^^^^^^^^ [warn]: Methods 'opCmp', 'toHash', 'opEquals', 'opCast', and/or 'toString' are non-const. +/
{
return "Dog";
}

View File

@ -5,12 +5,13 @@
module dscanner.analysis.opequals_without_tohash;
import std.stdio;
import dparse.ast;
import dparse.lexer;
import dscanner.analysis.base;
import dscanner.analysis.helpers;
import dsymbol.scope_ : Scope;
import std.stdio;
import std.typecons : Rebindable;
/**
* Checks for when a class/struct has the method opEquals without toHash, or
@ -22,9 +23,9 @@ final class OpEqualsWithoutToHashCheck : BaseAnalyzer
mixin AnalyzerInfo!"opequals_tohash_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const ClassDeclaration node)
@ -41,9 +42,9 @@ final class OpEqualsWithoutToHashCheck : BaseAnalyzer
private void actualCheck(const Token name, const StructBody structBody)
{
bool hasOpEquals;
bool hasToHash;
bool hasOpCmp;
Rebindable!(const Declaration) hasOpEquals;
Rebindable!(const Declaration) hasToHash;
Rebindable!(const Declaration) hasOpCmp;
// Just return if missing children
if (!structBody || !structBody.declarations || name is Token.init)
@ -72,24 +73,36 @@ final class OpEqualsWithoutToHashCheck : BaseAnalyzer
// Check if opEquals or toHash
immutable string methodName = declaration.functionDeclaration.name.text;
if (methodName == "opEquals")
hasOpEquals = true;
hasOpEquals = declaration;
else if (methodName == "toHash")
hasToHash = true;
hasToHash = declaration;
else if (methodName == "opCmp")
hasOpCmp = true;
hasOpCmp = declaration;
}
// Warn if has opEquals, but not toHash
if (hasOpEquals && !hasToHash)
{
string message = "'" ~ name.text ~ "' has method 'opEquals', but not 'toHash'.";
addErrorMessage(name.line, name.column, KEY, message);
addErrorMessage(
Message.Diagnostic.from(fileName, name, message),
[
Message.Diagnostic.from(fileName, hasOpEquals.get, "'opEquals' defined here")
],
KEY
);
}
// Warn if has toHash, but not opEquals
else if (!hasOpEquals && hasToHash)
{
string message = "'" ~ name.text ~ "' has method 'toHash', but not 'opEquals'.";
addErrorMessage(name.line, name.column, KEY, message);
addErrorMessage(
Message.Diagnostic.from(fileName, name, message),
[
Message.Diagnostic.from(fileName, hasToHash.get, "'toHash' defined here")
],
KEY
);
}
}
@ -102,6 +115,7 @@ unittest
StaticAnalysisConfig sac = disabledConfig();
sac.opequals_tohash_check = Check.enabled;
// TODO: test supplemental diagnostics
assertAnalyzerWarnings(q{
// Success because it has opEquals and toHash
class Chimp
@ -127,7 +141,8 @@ unittest
}
// Fail on class opEquals
class Rabbit // [warn]: 'Rabbit' has method 'opEquals', but not 'toHash'.
class Rabbit /+
^^^^^^ [warn]: 'Rabbit' has method 'opEquals', but not 'toHash'. +/
{
const bool opEquals(Object a, Object b)
{
@ -136,7 +151,8 @@ unittest
}
// Fail on class toHash
class Kangaroo // [warn]: 'Kangaroo' has method 'toHash', but not 'opEquals'.
class Kangaroo /+
^^^^^^^^ [warn]: 'Kangaroo' has method 'toHash', but not 'opEquals'. +/
{
override const hash_t toHash()
{
@ -145,7 +161,8 @@ unittest
}
// Fail on struct opEquals
struct Tarantula // [warn]: 'Tarantula' has method 'opEquals', but not 'toHash'.
struct Tarantula /+
^^^^^^^^^ [warn]: 'Tarantula' has method 'opEquals', but not 'toHash'. +/
{
const bool opEquals(Object a, Object b)
{
@ -154,7 +171,8 @@ unittest
}
// Fail on struct toHash
struct Puma // [warn]: 'Puma' has method 'toHash', but not 'opEquals'.
struct Puma /+
^^^^ [warn]: 'Puma' has method 'toHash', but not 'opEquals'. +/
{
const nothrow @safe hash_t toHash()
{

View File

@ -31,14 +31,14 @@ final class PokemonExceptionCheck : BaseAnalyzer
alias visit = BaseAnalyzer.visit;
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const LastCatch lc)
{
addErrorMessage(lc.line, lc.column, KEY, MESSAGE);
addErrorMessage(lc.tokens[0], KEY, MESSAGE);
lc.accept(this);
}
@ -76,9 +76,7 @@ final class PokemonExceptionCheck : BaseAnalyzer
if (identOrTemplate.identifier.text == "Throwable"
|| identOrTemplate.identifier.text == "Error")
{
immutable column = identOrTemplate.identifier.column;
immutable line = identOrTemplate.identifier.line;
addErrorMessage(line, column, KEY, MESSAGE);
addErrorMessage(identOrTemplate, KEY, MESSAGE);
}
}
}
@ -108,19 +106,23 @@ unittest
{
}
catch (Error err) // [warn]: Catching Error or Throwable is almost always a bad idea.
catch (Error err) /+
^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/
{
}
catch (Throwable err) // [warn]: Catching Error or Throwable is almost always a bad idea.
catch (Throwable err) /+
^^^^^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/
{
}
catch (shared(Error) err) // [warn]: Catching Error or Throwable is almost always a bad idea.
catch (shared(Error) err) /+
^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/
{
}
catch // [warn]: Catching Error or Throwable is almost always a bad idea.
catch /+
^^^^^ [warn]: Catching Error or Throwable is almost always a bad idea. +/
{
}

View File

@ -41,9 +41,9 @@ final class ProperlyDocumentedPublicFunctions : BaseAnalyzer
mixin AnalyzerInfo!"properly_documented_public_functions";
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const Module mod)
@ -91,8 +91,8 @@ final class ProperlyDocumentedPublicFunctions : BaseAnalyzer
thrown ~= newNamedType(tsa.token);
}
// enforce!(Type)(condition);
else if (const TemplateArgumentList tal = safeAccess(iot.templateInstance)
.templateArguments.templateArgumentList)
else if (const NamedTemplateArgumentList tal = safeAccess(iot.templateInstance)
.templateArguments.namedTemplateArgumentList)
{
if (tal.items.length && tal.items[0].type)
thrown ~= tal.items[0].type;
@ -157,8 +157,8 @@ final class ProperlyDocumentedPublicFunctions : BaseAnalyzer
override void visit(const TemplateDeclaration decl)
{
setLastDdocParams(decl.name.line, decl.name.column, decl.comment);
checkDdocParams(decl.name.line, decl.name.column, decl.templateParameters);
setLastDdocParams(decl.name, decl.comment);
checkDdocParams(decl.templateParameters);
withinTemplate = true;
scope(exit) withinTemplate = false;
@ -172,15 +172,15 @@ final class ProperlyDocumentedPublicFunctions : BaseAnalyzer
override void visit(const StructDeclaration decl)
{
setLastDdocParams(decl.name.line, decl.name.column, decl.comment);
checkDdocParams(decl.name.line, decl.name.column, decl.templateParameters);
setLastDdocParams(decl.name, decl.comment);
checkDdocParams(decl.templateParameters);
decl.accept(this);
}
override void visit(const ClassDeclaration decl)
{
setLastDdocParams(decl.name.line, decl.name.column, decl.comment);
checkDdocParams(decl.name.line, decl.name.column, decl.templateParameters);
setLastDdocParams(decl.name, decl.comment);
checkDdocParams(decl.templateParameters);
decl.accept(this);
}
@ -205,20 +205,31 @@ final class ProperlyDocumentedPublicFunctions : BaseAnalyzer
{
Appender!(char[]) app;
astFmt(&app, t);
addErrorMessage(decl.name.line, decl.name.column, MISSING_THROW_KEY,
addErrorMessage(decl.name, MISSING_THROW_KEY,
MISSING_THROW_MESSAGE.format(app.data));
}
}
if (nestedFuncs == 1)
{
auto comment = setLastDdocParams(decl.name.line, decl.name.column, decl.comment);
checkDdocParams(decl.name.line, decl.name.column, decl.parameters, decl.templateParameters);
auto comment = setLastDdocParams(decl.name, decl.comment);
checkDdocParams(decl.parameters, decl.templateParameters);
enum voidType = tok!"void";
if (decl.returnType is null || decl.returnType.type2.builtinType != voidType)
if (!(comment.isDitto || withinTemplate || comment.sections.any!(s => s.name == "Returns")))
addErrorMessage(decl.name.line, decl.name.column,
MISSING_RETURNS_KEY, MISSING_RETURNS_MESSAGE);
{
import dscanner.analysis.auto_function : AutoFunctionChecker;
const(Token)[] typeRange;
if (decl.returnType !is null)
typeRange = decl.returnType.tokens;
else
typeRange = AutoFunctionChecker.findAutoReturnType(decl);
if (!typeRange.length)
typeRange = [decl.name];
addErrorMessage(typeRange, MISSING_RETURNS_KEY, MISSING_RETURNS_MESSAGE);
}
}
}
@ -257,7 +268,7 @@ private:
static struct Function
{
bool active;
size_t line, column;
Token name;
const(string)[] ddocParams;
bool[string] params;
}
@ -274,7 +285,7 @@ private:
if (lastSeenFun.active)
foreach (p; lastSeenFun.ddocParams)
if (p !in lastSeenFun.params)
addErrorMessage(lastSeenFun.line, lastSeenFun.column, NON_EXISTENT_PARAMS_KEY,
addErrorMessage(lastSeenFun.name, NON_EXISTENT_PARAMS_KEY,
NON_EXISTENT_PARAMS_MESSAGE.format(p));
lastSeenFun.active = false;
@ -289,7 +300,7 @@ private:
return comment.isDitto || comment.sections.canFind!(s => s.name == "Throws");
}
auto setLastDdocParams(size_t line, size_t column, string commentText)
auto setLastDdocParams(Token name, string commentText)
{
import ddoc.comments : parseComment;
import std.algorithm.searching : find;
@ -312,19 +323,19 @@ private:
const paramSection = comment.sections.find!(s => s.name == "Params");
if (paramSection.empty)
{
lastSeenFun = Function(true, line, column, null);
lastSeenFun = Function(true, name, null);
}
else
{
auto ddocParams = paramSection[0].mapping.map!(a => a[0]).array;
lastSeenFun = Function(true, line, column, ddocParams);
lastSeenFun = Function(true, name, ddocParams);
}
}
return comment;
}
void checkDdocParams(size_t line, size_t column, const Parameters params,
void checkDdocParams(const Parameters params,
const TemplateParameters templateParameters = null)
{
import std.array : array;
@ -338,7 +349,7 @@ private:
if (const tp = templateParameters)
if (const tpl = tp.templateParameterList)
templateList = tpl.items;
string[] tlList = templateList.map!(a => templateParamName(a)).array;
string[] tlList = templateList.map!(a => templateParamName(a).text).array;
// make a copy of all parameters and remove the seen ones later during the loop
size_t[] unseenTemplates = templateList.length.iota.array;
@ -369,7 +380,7 @@ private:
}
if (!lastSeenFun.ddocParams.canFind(p.name.text))
addErrorMessage(line, column, MISSING_PARAMS_KEY,
addErrorMessage(p.name, MISSING_PARAMS_KEY,
format(MISSING_PARAMS_MESSAGE, p.name.text));
else
lastSeenFun.params[p.name.text] = true;
@ -377,45 +388,45 @@ private:
// now check the remaining, not used template parameters
auto unseenTemplatesArr = templateList.indexed(unseenTemplates).array;
checkDdocParams(line, column, unseenTemplatesArr);
checkDdocParams(unseenTemplatesArr);
}
void checkDdocParams(size_t line, size_t column, const TemplateParameters templateParams)
void checkDdocParams(const TemplateParameters templateParams)
{
if (lastSeenFun.active && templateParams !is null &&
templateParams.templateParameterList !is null)
checkDdocParams(line, column, templateParams.templateParameterList.items);
checkDdocParams(templateParams.templateParameterList.items);
}
void checkDdocParams(size_t line, size_t column, const TemplateParameter[] templateParams)
void checkDdocParams(const TemplateParameter[] templateParams)
{
import std.algorithm.searching : canFind;
foreach (p; templateParams)
{
const name = templateParamName(p);
assert(name, "Invalid template parameter name."); // this shouldn't happen
if (!lastSeenFun.ddocParams.canFind(name))
addErrorMessage(line, column, MISSING_PARAMS_KEY,
format(MISSING_TEMPLATE_PARAMS_MESSAGE, name));
assert(name !is Token.init, "Invalid template parameter name."); // this shouldn't happen
if (!lastSeenFun.ddocParams.canFind(name.text))
addErrorMessage(name, MISSING_PARAMS_KEY,
format(MISSING_TEMPLATE_PARAMS_MESSAGE, name.text));
else
lastSeenFun.params[name] = true;
lastSeenFun.params[name.text] = true;
}
}
static string templateParamName(const TemplateParameter p)
static Token templateParamName(const TemplateParameter p)
{
if (p.templateTypeParameter)
return p.templateTypeParameter.identifier.text;
return p.templateTypeParameter.identifier;
if (p.templateValueParameter)
return p.templateValueParameter.identifier.text;
return p.templateValueParameter.identifier;
if (p.templateAliasParameter)
return p.templateAliasParameter.identifier.text;
return p.templateAliasParameter.identifier;
if (p.templateTupleParameter)
return p.templateTupleParameter.identifier.text;
return p.templateTupleParameter.identifier;
if (p.templateThisParameter)
return p.templateThisParameter.templateTypeParameter.identifier.text;
return p.templateThisParameter.templateTypeParameter.identifier;
return null;
return Token.init;
}
bool hasDeclaration(const Declaration decl)
@ -483,7 +494,8 @@ unittest
/**
Some text
*/
void foo(int k){} // [warn]: %s
void foo(int k){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("k")
), sac);
@ -492,7 +504,8 @@ unittest
/**
Some text
*/
void foo(int K)(){} // [warn]: %s
void foo(int K)(){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("K")
), sac);
@ -501,7 +514,8 @@ unittest
/**
Some text
*/
struct Foo(Bar){} // [warn]: %s
struct Foo(Bar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("Bar")
), sac);
@ -510,7 +524,8 @@ unittest
/**
Some text
*/
class Foo(Bar){} // [warn]: %s
class Foo(Bar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("Bar")
), sac);
@ -519,7 +534,8 @@ unittest
/**
Some text
*/
template Foo(Bar){} // [warn]: %s
template Foo(Bar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("Bar")
), sac);
@ -558,7 +574,8 @@ unittest
/**
Some text
*/
int foo(){} // [warn]: %s
int foo(){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_RETURNS_MESSAGE,
), sac);
@ -567,7 +584,8 @@ unittest
/**
Some text
*/
auto foo(){} // [warn]: %s
auto foo(){} /+
^^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_RETURNS_MESSAGE,
), sac);
@ -594,10 +612,12 @@ unittest
*/
private void foo(int k){}
///
public int bar(){} // [warn]: %s
public int bar(){} /+
^^^ [warn]: %s +/
public:
///
int foobar(){} // [warn]: %s
int foobar(){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_RETURNS_MESSAGE,
ProperlyDocumentedPublicFunctions.MISSING_RETURNS_MESSAGE,
@ -611,10 +631,12 @@ unittest
*/
private template foo(int k){}
///
public template bar(T){} // [warn]: %s
public template bar(T){} /+
^ [warn]: %s +/
public:
///
template foobar(T){} // [warn]: %s
template foobar(T){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("T"),
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("T"),
@ -628,10 +650,12 @@ unittest
*/
private struct foo(int k){}
///
public struct bar(T){} // [warn]: %s
public struct bar(T){} /+
^ [warn]: %s +/
public:
///
struct foobar(T){} // [warn]: %s
struct foobar(T){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("T"),
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("T"),
@ -653,7 +677,8 @@ unittest
* Returns:
* A long description.
*/
int foo(int k){} // [warn]: %s
int foo(int k){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("k")
), sac);
@ -667,7 +692,8 @@ int foo(int k){} // [warn]: %s
* Returns:
* A long description.
*/
int foo(int k) => k; // [warn]: %s
int foo(int k) => k; /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("k")
), sac);
@ -683,7 +709,8 @@ k = A stupid parameter
Returns:
A long description.
*/
int foo(int k){} // [warn]: %s
int foo(int k){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.NON_EXISTENT_PARAMS_MESSAGE.format("val")
), sac);
@ -697,7 +724,8 @@ Params:
Returns:
A long description.
*/
int foo(int k){} // [warn]: %s
int foo(int k){} /+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("k")
), sac);
@ -714,7 +742,8 @@ foobar = A stupid parameter
Returns:
A long description.
*/
int foo(int foo, int foobar){} // [warn]: %s
int foo(int foo, int foobar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.NON_EXISTENT_PARAMS_MESSAGE.format("bad")
), sac);
@ -731,7 +760,8 @@ foobar = A stupid parameter
Returns:
A long description.
*/
struct foo(int foo, int foobar){} // [warn]: %s
struct foo(int foo, int foobar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.NON_EXISTENT_PARAMS_MESSAGE.format("bad")
), sac);
@ -835,7 +865,8 @@ int bar(int f){}
int foo(int k){}
/// ditto
int bar(int bar){} // [warn]: %s
int bar(int bar){} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_PARAMS_MESSAGE.format("bar")
), sac);
@ -854,7 +885,8 @@ int bar(int bar){} // [warn]: %s
* See_Also:
* $(REF takeExactly, std,range)
*/
int foo(int k){} // [warn]: %s
int foo(int k){} /+
^^^ [warn]: %s +/
/// ditto
int bar(int bar){}
@ -956,7 +988,8 @@ Params:
Returns: Awesome values.
+/
string bar(P, R)(R r){}// [warn]: %s
string bar(P, R)(R r){}/+
^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_TEMPLATE_PARAMS_MESSAGE.format("P")
), sac);
@ -1020,7 +1053,8 @@ unittest
/++
Simple case
+/
void bar(){throw new Exception("bla");} // [warn]: %s
void bar(){throw new Exception("bla");} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("Exception")
), sac);
@ -1051,7 +1085,8 @@ unittest
/++
rethrow
+/
void bar(){try throw new Exception("bla"); catch(Exception) throw new Error();} // [warn]: %s
void bar(){try throw new Exception("bla"); catch(Exception) throw new Error();} /+
^^^ [warn]: %s +/
}c.format(
ProperlyDocumentedPublicFunctions.MISSING_THROW_MESSAGE.format("Error")
), sac);
@ -1079,7 +1114,8 @@ unittest
/++
case of throw in nested func
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
void foo(){throw new AssertError("bla");}
foo();
@ -1116,7 +1152,8 @@ unittest
/++
case of double throw in nested func but only 1 caught
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
void foo(){throw new AssertError("bla");throw new Error("bla");}
try foo();
@ -1136,7 +1173,8 @@ unittest
/++
enforce
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
enforce(condition);
}
@ -1154,7 +1192,8 @@ unittest
/++
enforce
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
enforce!AssertError(condition);
}
@ -1172,7 +1211,8 @@ unittest
/++
enforce
+/
void bar() // [warn]: %s
void bar() /+
^^^ [warn]: %s +/
{
enforce!(AssertError)(condition);
}
@ -1190,7 +1230,8 @@ unittest
/++
enforce
+/
void foo() // [warn]: %s
void foo() /+
^^^ [warn]: %s +/
{
void bar()
{

View File

@ -29,9 +29,9 @@ final class BackwardsRangeCheck : BaseAnalyzer
* Params:
* fileName = the name of the file being analyzed
*/
this(string fileName, const Scope* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const ForeachStatement foreachStatement)
@ -50,7 +50,11 @@ final class BackwardsRangeCheck : BaseAnalyzer
string message = format(
"%d is larger than %d. Did you mean to use 'foreach_reverse( ... ; %d .. %d)'?",
left, right, right, left);
addErrorMessage(line, this.column, KEY, message);
auto start = &foreachStatement.low.tokens[0];
auto endIncl = &foreachStatement.high.tokens[$ - 1];
assert(endIncl >= start);
auto tokens = start[0 .. endIncl - start + 1];
addErrorMessage(tokens, KEY, message);
}
hasLeft = false;
hasRight = false;
@ -82,9 +86,6 @@ final class BackwardsRangeCheck : BaseAnalyzer
return;
if (state == State.left)
{
line = primary.primary.line;
this.column = primary.primary.column;
try
left = parseNumber(primary.primary.text);
catch (ConvException e)
@ -106,9 +107,9 @@ final class BackwardsRangeCheck : BaseAnalyzer
if (index.low !is null && index.high !is null)
{
state = State.left;
visit(index.low);
dynamicDispatch(index.low);
state = State.right;
visit(index.high);
dynamicDispatch(index.high);
state = State.ignore;
if (hasLeft && hasRight && left > right)
{
@ -116,7 +117,7 @@ final class BackwardsRangeCheck : BaseAnalyzer
string message = format("%d is larger than %d. This slice is likely incorrect.",
left, right);
addErrorMessage(line, this.column, KEY, message);
addErrorMessage(index, KEY, message);
}
hasLeft = false;
hasRight = false;
@ -129,8 +130,6 @@ private:
bool hasRight;
long left;
long right;
size_t column;
size_t line;
enum State
{
ignore,
@ -173,10 +172,12 @@ unittest
int[] data = [1, 2, 3, 4, 5];
data = data[1 .. 3]; // ok
data = data[3 .. 1]; // [warn]: 3 is larger than 1. This slice is likely incorrect.
data = data[3 .. 1]; /+
^^^^^^ [warn]: 3 is larger than 1. This slice is likely incorrect. +/
foreach (n; 1 .. 3) { } // ok
foreach (n; 3 .. 1) { } // [warn]: 3 is larger than 1. Did you mean to use 'foreach_reverse( ... ; 1 .. 3)'?
foreach (n; 3 .. 1) { } /+
^^^^^^ [warn]: 3 is larger than 1. Did you mean to use 'foreach_reverse( ... ; 1 .. 3)'? +/
}
}c, sac);

View File

@ -17,13 +17,13 @@ import std.range : empty, front, walkLength;
/**
* Checks for redundant attributes. At the moment only visibility attributes.
*/
final class RedundantAttributesCheck : BaseAnalyzer
final class RedundantAttributesCheck : ScopedBaseAnalyzer
{
mixin AnalyzerInfo!"redundant_attributes_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
stack.length = 0;
}
@ -67,15 +67,8 @@ final class RedundantAttributesCheck : BaseAnalyzer
}
}
alias visit = BaseAnalyzer.visit;
alias visit = ScopedBaseAnalyzer.visit;
mixin ScopedVisit!Module;
mixin ScopedVisit!BlockStatement;
mixin ScopedVisit!StructBody;
mixin ScopedVisit!CaseStatement;
mixin ScopedVisit!ForStatement;
mixin ScopedVisit!IfStatement;
mixin ScopedVisit!TemplateDeclaration;
mixin ScopedVisit!ConditionalDeclaration;
private:
@ -103,8 +96,7 @@ private:
auto match = currentAttributes.find!(a => a.attribute.type == attr.attribute.type);
if (!match.empty)
{
auto token = attr.attribute;
addErrorMessage(token.line, token.column, KEY,
addErrorMessage(attr, KEY,
text("same visibility attribute used as defined on line ",
match.front.attribute.line.to!string, "."));
return false;
@ -154,22 +146,12 @@ private:
return currentAttributes.map!(a => a.attribute.type.str).joiner(",").to!string;
}
template ScopedVisit(NodeType)
{
override void visit(const NodeType n)
{
pushScope();
n.accept(this);
popScope();
}
}
void pushScope()
protected override void pushScope()
{
stack.length++;
}
void popScope()
protected override void popScope()
{
stack.length--;
}
@ -194,12 +176,15 @@ unittest
unittest
{
private:
private int blah; // [warn]: same visibility attribute used as defined on line 4.
private int blah; /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
protected
{
protected int blah; // [warn]: same visibility attribute used as defined on line 6.
protected int blah; /+
^^^^^^^^^ [warn]: same visibility attribute used as defined on line 7. +/
}
private int blah; // [warn]: same visibility attribute used as defined on line 4.
private int blah; /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
}}c, sac);
// test labels vs. block attributes
@ -207,11 +192,14 @@ protected
unittest
{
private:
private: // [warn]: same visibility attribute used as defined on line 4.
private: /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
public:
private int a;
public int b; // [warn]: same visibility attribute used as defined on line 6.
public // [warn]: same visibility attribute used as defined on line 6.
public int b; /+
^^^^^^ [warn]: same visibility attribute used as defined on line 7. +/
public /+
^^^^^^ [warn]: same visibility attribute used as defined on line 7. +/
{
int c;
}
@ -222,8 +210,10 @@ unittest
unittest
{
private:
private int foo2; // [warn]: same visibility attribute used as defined on line 4.
private void foo() // [warn]: same visibility attribute used as defined on line 4.
private int foo2; /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
private void foo() /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
{
private int blah;
}
@ -235,7 +225,8 @@ unittest
{
private:
public int a;
private: // [warn]: same visibility attribute used as defined on line 4.
private: /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
}}c, sac);
// test conditional compilation
@ -245,7 +236,8 @@ unittest
version(unittest)
{
private:
private int foo; // [warn]: same visibility attribute used as defined on line 6.
private int foo; /+
^^^^^^^ [warn]: same visibility attribute used as defined on line 6. +/
}
private int foo2;
}}c, sac);
@ -263,7 +255,8 @@ public:
{
public int b;
}
public int b; // [warn]: same visibility attribute used as defined on line 4.
public int b; /+
^^^^^^ [warn]: same visibility attribute used as defined on line 4. +/
}}c, sac);
}

View File

@ -20,25 +20,24 @@ final class RedundantParenCheck : BaseAnalyzer
mixin AnalyzerInfo!"redundant_parens_check";
///
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const IfStatement statement)
{
UnaryExpression unary;
if (statement.expression is null || statement.expression.items.length != 1)
if (statement.condition.expression is null || statement.condition.expression.items.length != 1)
goto end;
unary = cast(UnaryExpression) statement.expression.items[0];
unary = cast(UnaryExpression) statement.condition.expression.items[0];
if (unary is null)
goto end;
if (unary.primaryExpression is null)
goto end;
if (unary.primaryExpression.expression is null)
goto end;
addErrorMessage(unary.primaryExpression.expression.line,
unary.primaryExpression.expression.column, KEY, "Redundant parenthesis.");
addErrorMessage(unary.primaryExpression, KEY, "Redundant parenthesis.");
end:
statement.accept(this);
}
@ -55,8 +54,7 @@ final class RedundantParenCheck : BaseAnalyzer
goto end;
if (unary.primaryExpression.expression is null)
goto end;
addErrorMessage(primaryExpression.expression.line,
primaryExpression.expression.column, KEY, "Redundant parenthesis.");
addErrorMessage(primaryExpression, KEY, "Redundant parenthesis.");
end:
primaryExpression.accept(this);
}

View File

@ -22,9 +22,9 @@ final class RedundantStorageClassCheck : BaseAnalyzer
enum string REDUNDANT_VARIABLE_ATTRIBUTES = "Variable declaration for `%s` has redundant attributes (%-(`%s`%|, %)).";
mixin AnalyzerInfo!"redundant_storage_classes";
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const Declaration node)
@ -59,9 +59,11 @@ final class RedundantStorageClassCheck : BaseAnalyzer
return;
auto t = vd.declarators[0].name;
string message = REDUNDANT_VARIABLE_ATTRIBUTES.format(t.text, globalAttributes);
addErrorMessage(t.line, t.column, "dscanner.unnecessary.duplicate_attribute", message);
addErrorMessage(t, KEY, message);
}
}
private enum string KEY = "dscanner.unnecessary.duplicate_attribute";
}
unittest

File diff suppressed because it is too large Load Diff

View File

@ -19,7 +19,7 @@ import dscanner.utils : safeAccess;
* } else if (bar) {
* }
* ---
*
*
* However, it's more likely that this is a mistake.
*/
final class StaticIfElse : BaseAnalyzer
@ -28,9 +28,9 @@ final class StaticIfElse : BaseAnalyzer
mixin AnalyzerInfo!"static_if_else_check";
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const ConditionalStatement cc)
@ -45,7 +45,14 @@ final class StaticIfElse : BaseAnalyzer
{
return;
}
addErrorMessage(ifStmt.line, ifStmt.column, KEY, "Mismatched static if. Use 'else static if' here.");
auto tokens = ifStmt.tokens[0 .. 1];
// extend one token to include `else` before this if
tokens = (tokens.ptr - 1)[0 .. 2];
addErrorMessage(tokens, KEY, "Mismatched static if. Use 'else static if' here.",
[
AutoFix.insertionBefore(tokens[$ - 1], "static "),
AutoFix.resolveLater("Wrap '{}' block around 'if'", [tokens[0].index, ifStmt.tokens[$ - 1].index, 0])
]);
}
const(IfStatement) getIfStatement(const ConditionalStatement cc)
@ -53,13 +60,40 @@ final class StaticIfElse : BaseAnalyzer
return safeAccess(cc).falseStatement.statement.statementNoCaseNoDefault.ifStatement;
}
override AutoFix.CodeReplacement[] resolveAutoFix(
const Module mod,
scope const(Token)[] tokens,
const AutoFix.ResolveContext context,
const AutoFixFormatting formatting,
)
{
import dscanner.analysis.helpers : getLineIndentation;
import std.algorithm : countUntil;
auto beforeElse = tokens.countUntil!(a => a.index == context.params[0]);
auto lastToken = tokens.countUntil!(a => a.index == context.params[1]);
if (beforeElse == -1 || lastToken == -1)
throw new Exception("got different tokens than what was used to generate this autofix");
auto indentation = getLineIndentation(tokens, tokens[beforeElse].line, formatting);
string beforeIf = formatting.getWhitespaceBeforeOpeningBrace(indentation, false)
~ "{" ~ formatting.eol ~ indentation;
string afterIf = formatting.eol ~ indentation ~ "}";
return AutoFix.replacement([tokens[beforeElse].index + 4, tokens[beforeElse + 1].index], beforeIf, "")
.concat(AutoFix.indentLines(tokens[beforeElse + 1 .. lastToken + 1], formatting))
.concat(AutoFix.insertionAfter(tokens[lastToken], afterIf))
.expectReplacements;
}
enum KEY = "dscanner.suspicious.static_if_else";
}
unittest
{
import dscanner.analysis.helpers : assertAnalyzerWarnings;
import dscanner.analysis.config : StaticAnalysisConfig, Check, disabledConfig;
import dscanner.analysis.config : Check, disabledConfig, StaticAnalysisConfig;
import dscanner.analysis.helpers : assertAnalyzerWarnings, assertAutoFix;
import std.stdio : stderr;
StaticAnalysisConfig sac = disabledConfig();
@ -68,7 +102,8 @@ unittest
void foo() {
static if (false)
auto a = 0;
else if (true) // [warn]: Mismatched static if. Use 'else static if' here.
else if (true) /+
^^^^^^^ [warn]: Mismatched static if. Use 'else static if' here. +/
auto b = 1;
}
}c, sac);
@ -84,5 +119,51 @@ unittest
}
}c, sac);
assertAutoFix(q{
void foo() {
static if (false)
auto a = 0;
else if (true) // fix:0
auto b = 1;
}
void bar() {
static if (false)
auto a = 0;
else if (true) // fix:1
auto b = 1;
}
void baz() {
static if (false)
auto a = 0;
else if (true) { // fix:1
auto b = 1;
}
}
}c, q{
void foo() {
static if (false)
auto a = 0;
else static if (true) // fix:0
auto b = 1;
}
void bar() {
static if (false)
auto a = 0;
else {
if (true) // fix:1
auto b = 1;
}
}
void baz() {
static if (false)
auto a = 0;
else {
if (true) { // fix:1
auto b = 1;
}
}
}
}c, sac);
stderr.writeln("Unittest for StaticIfElse passed.");
}

View File

@ -13,9 +13,10 @@ final class StatsCollector : BaseAnalyzer
{
alias visit = ASTVisitor.visit;
this(string fileName)
this(BaseAnalyzerArguments args)
{
super(fileName, null);
args.skipTests = false; // old behavior compatibility
super(args);
}
override void visit(const Statement statement)

View File

@ -14,6 +14,7 @@ import std.conv;
import std.format;
import dscanner.analysis.helpers;
import dscanner.analysis.base;
import dscanner.analysis.nolint;
import dsymbol.scope_ : Scope;
final class StyleChecker : BaseAnalyzer
@ -26,17 +27,20 @@ final class StyleChecker : BaseAnalyzer
enum string KEY = "dscanner.style.phobos_naming_convention";
mixin AnalyzerInfo!"style_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const ModuleDeclaration dec)
{
with (noLint.push(NoLintFactory.fromModuleDeclaration(dec)))
dec.accept(this);
foreach (part; dec.moduleName.identifiers)
{
if (part.text.matchFirst(moduleNameRegex).length == 0)
addErrorMessage(part.line, part.column, KEY,
addErrorMessage(part, KEY,
"Module/package name '" ~ part.text ~ "' does not match style guidelines.");
}
}
@ -102,7 +106,7 @@ final class StyleChecker : BaseAnalyzer
void checkLowercaseName(string type, ref const Token name)
{
if (name.text.length > 0 && name.text.matchFirst(varFunNameRegex).length == 0)
addErrorMessage(name.line, name.column, KEY,
addErrorMessage(name, KEY,
type ~ " name '" ~ name.text ~ "' does not match style guidelines.");
}
@ -135,7 +139,7 @@ final class StyleChecker : BaseAnalyzer
void checkAggregateName(string aggregateType, ref const Token name)
{
if (name.text.length > 0 && name.text.matchFirst(aggregateNameRegex).length == 0)
addErrorMessage(name.line, name.column, KEY,
addErrorMessage(name, KEY,
aggregateType ~ " name '" ~ name.text ~ "' does not match style guidelines.");
}
@ -166,17 +170,22 @@ unittest
sac.style_check = Check.enabled;
assertAnalyzerWarnings(q{
module AMODULE; // [warn]: Module/package name 'AMODULE' does not match style guidelines.
module AMODULE; /+
^^^^^^^ [warn]: Module/package name 'AMODULE' does not match style guidelines. +/
bool A_VARIABLE; // FIXME:
bool a_variable; // ok
bool aVariable; // ok
void A_FUNCTION() {} // FIXME:
class cat {} // [warn]: Class name 'cat' does not match style guidelines.
interface puma {} // [warn]: Interface name 'puma' does not match style guidelines.
struct dog {} // [warn]: Struct name 'dog' does not match style guidelines.
enum racoon { a } // [warn]: Enum name 'racoon' does not match style guidelines.
class cat {} /+
^^^ [warn]: Class name 'cat' does not match style guidelines. +/
interface puma {} /+
^^^^ [warn]: Interface name 'puma' does not match style guidelines. +/
struct dog {} /+
^^^ [warn]: Struct name 'dog' does not match style guidelines. +/
enum racoon { a } /+
^^^^^^ [warn]: Enum name 'racoon' does not match style guidelines. +/
enum bool something = false;
enum bool someThing = false;
enum Cat { fritz, }
@ -194,7 +203,8 @@ unittest
assertAnalyzerWarnings(q{
extern(Windows)
{
extern(D) bool Fun2(); // [warn]: Function name 'Fun2' does not match style guidelines.
extern(D) bool Fun2(); /+
^^^^ [warn]: Function name 'Fun2' does not match style guidelines. +/
bool Fun3();
}
}c, sac);
@ -203,8 +213,10 @@ unittest
extern(Windows)
{
extern(C):
extern(D) bool Fun4(); // [warn]: Function name 'Fun4' does not match style guidelines.
bool Fun5(); // [warn]: Function name 'Fun5' does not match style guidelines.
extern(D) bool Fun4(); /+
^^^^ [warn]: Function name 'Fun4' does not match style guidelines. +/
bool Fun5(); /+
^^^^ [warn]: Function name 'Fun5' does not match style guidelines. +/
}
}c, sac);
@ -214,12 +226,14 @@ unittest
bool Fun7();
extern(D):
void okOkay();
void NotReallyOkay(); // [warn]: Function name 'NotReallyOkay' does not match style guidelines.
void NotReallyOkay(); /+
^^^^^^^^^^^^^ [warn]: Function name 'NotReallyOkay' does not match style guidelines. +/
}c, sac);
assertAnalyzerWarnings(q{
extern(Windows):
bool WinButWithBody(){} // [warn]: Function name 'WinButWithBody' does not match style guidelines.
bool WinButWithBody(){} /+
^^^^^^^^^^^^^^ [warn]: Function name 'WinButWithBody' does not match style guidelines. +/
}c, sac);
stderr.writeln("Unittest for StyleChecker passed.");

View File

@ -31,18 +31,15 @@ public:
mixin AnalyzerInfo!"trust_too_much";
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const AtAttribute d)
{
if (checkAtAttribute && d.identifier.text == "trusted")
{
const Token t = d.identifier;
addErrorMessage(t.line, t.column, KEY, MESSAGE);
}
addErrorMessage(d, KEY, MESSAGE);
d.accept(this);
}
@ -91,17 +88,20 @@ unittest
//--- fail cases ---//
assertAnalyzerWarnings(q{
@trusted: // [warn]: %s
@trusted: /+
^^^^^^^^ [warn]: %s +/
void test();
}c.format(msg), sac);
assertAnalyzerWarnings(q{
@trusted @nogc: // [warn]: %s
@trusted @nogc: /+
^^^^^^^^ [warn]: %s +/
void test();
}c.format(msg), sac);
assertAnalyzerWarnings(q{
@trusted { // [warn]: %s
@trusted { /+
^^^^^^^^ [warn]: %s +/
void test();
void test();
}
@ -109,27 +109,31 @@ unittest
assertAnalyzerWarnings(q{
@safe {
@trusted @nogc { // [warn]: %s
@trusted @nogc { /+
^^^^^^^^ [warn]: %s +/
void test();
void test();
}}
}c.format(msg), sac);
assertAnalyzerWarnings(q{
@nogc @trusted { // [warn]: %s
@nogc @trusted { /+
^^^^^^^^ [warn]: %s +/
void test();
void test();
}
}c.format(msg), sac);
assertAnalyzerWarnings(q{
@trusted template foo(){ // [warn]: %s
@trusted template foo(){ /+
^^^^^^^^ [warn]: %s +/
}
}c.format(msg), sac);
assertAnalyzerWarnings(q{
struct foo{
@trusted: // [warn]: %s
@trusted: /+
^^^^^^^^ [warn]: %s +/
}
}c.format(msg), sac);
//--- pass cases ---//

View File

@ -23,9 +23,9 @@ final class UndocumentedDeclarationCheck : BaseAnalyzer
mixin AnalyzerInfo!"undocumented_declaration_check";
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const Module mod)
@ -100,15 +100,14 @@ final class UndocumentedDeclarationCheck : BaseAnalyzer
return;
if (variable.autoDeclaration !is null)
{
addMessage(variable.autoDeclaration.parts[0].identifier.line,
variable.autoDeclaration.parts[0].identifier.column,
addMessage(variable.autoDeclaration.parts[0].identifier,
variable.autoDeclaration.parts[0].identifier.text);
return;
}
foreach (dec; variable.declarators)
{
if (dec.comment.ptr is null)
addMessage(dec.name.line, dec.name.column, dec.name.text);
addMessage(dec.name, dec.name.text);
return;
}
}
@ -147,6 +146,8 @@ final class UndocumentedDeclarationCheck : BaseAnalyzer
private:
enum string KEY = "dscanner.style.undocumented_declaration";
mixin template V(T)
{
override void visit(const T declaration)
@ -174,20 +175,25 @@ private:
|| isGetterOrSetter(declaration.name.text)
|| isProperty(declaration)))
{
addMessage(declaration.name.line,
declaration.name.column, declaration.name.text);
addMessage(declaration.name, declaration.name.text);
}
}
else
{
if (declaration.name.type != tok!"")
addMessage(declaration.name.line,
declaration.name.column, declaration.name.text);
addMessage(declaration.name, declaration.name.text);
}
}
else
{
addMessage(declaration.line, declaration.column, null);
import std.algorithm : countUntil;
// like constructors
auto tokens = declaration.tokens.findTokenForDisplay(tok!"this");
auto earlyEnd = tokens.countUntil!(a => a == tok!"{" || a == tok!"(" || a == tok!";");
if (earlyEnd != -1)
tokens = tokens[0 .. earlyEnd];
addMessage(tokens, null);
}
}
static if (!(is(T == TemplateDeclaration) || is(T == FunctionDeclaration)))
@ -215,11 +221,11 @@ private:
return false;
}
void addMessage(size_t line, size_t column, string name)
void addMessage(T)(T range, string name)
{
import std.string : format;
addErrorMessage(line, column, "dscanner.style.undocumented_declaration", name is null
addErrorMessage(range, KEY, name is null
? "Public declaration is undocumented."
: format("Public declaration '%s' is undocumented.", name));
}
@ -315,13 +321,20 @@ unittest
sac.undocumented_declaration_check = Check.enabled;
assertAnalyzerWarnings(q{
class C{} // [warn]: Public declaration 'C' is undocumented.
interface I{} // [warn]: Public declaration 'I' is undocumented.
enum e = 0; // [warn]: Public declaration 'e' is undocumented.
void f(){} // [warn]: Public declaration 'f' is undocumented.
struct S{} // [warn]: Public declaration 'S' is undocumented.
template T(){} // [warn]: Public declaration 'T' is undocumented.
union U{} // [warn]: Public declaration 'U' is undocumented.
class C{} /+
^ [warn]: Public declaration 'C' is undocumented. +/
interface I{} /+
^ [warn]: Public declaration 'I' is undocumented. +/
enum e = 0; /+
^ [warn]: Public declaration 'e' is undocumented. +/
void f(){} /+
^ [warn]: Public declaration 'f' is undocumented. +/
struct S{} /+
^ [warn]: Public declaration 'S' is undocumented. +/
template T(){} /+
^ [warn]: Public declaration 'T' is undocumented. +/
union U{} /+
^ [warn]: Public declaration 'U' is undocumented. +/
}, sac);
assertAnalyzerWarnings(q{

View File

@ -5,6 +5,7 @@
module dscanner.analysis.unmodified;
import dscanner.analysis.base;
import dscanner.analysis.nolint;
import dscanner.utils : safeAccess;
import dsymbol.scope_ : Scope;
import std.container;
@ -21,9 +22,9 @@ final class UnmodifiedFinder : BaseAnalyzer
mixin AnalyzerInfo!"could_be_immutable_check";
///
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const Module mod)
@ -63,8 +64,7 @@ final class UnmodifiedFinder : BaseAnalyzer
continue;
if (initializedFromNew(d.initializer))
continue;
tree[$ - 1].insert(new VariableInfo(d.name.text, d.name.line,
d.name.column, isValueTypeSimple(dec.type)));
tree[$ - 1].insert(new VariableInfo(d.name.text, d.name, isValueTypeSimple(dec.type)));
}
}
dec.accept(this);
@ -84,8 +84,7 @@ final class UnmodifiedFinder : BaseAnalyzer
continue;
if (initializedFromNew(part.initializer))
continue;
tree[$ - 1].insert(new VariableInfo(part.identifier.text,
part.identifier.line, part.identifier.column));
tree[$ - 1].insert(new VariableInfo(part.identifier.text, part.identifier));
}
}
autoDeclaration.accept(this);
@ -116,11 +115,15 @@ final class UnmodifiedFinder : BaseAnalyzer
if (canFindImmutableOrConst(dec))
{
isImmutable++;
dec.accept(this);
with (noLint.push(NoLintFactory.fromDeclaration(dec)))
dec.accept(this);
isImmutable--;
}
else
dec.accept(this);
{
with (noLint.push(NoLintFactory.fromDeclaration(dec)))
dec.accept(this);
}
}
override void visit(const IdentifierChain ic)
@ -191,6 +194,8 @@ final class UnmodifiedFinder : BaseAnalyzer
private:
enum string KEY = "dscanner.suspicious.unmodified";
template PartsMightModify(T)
{
override void visit(const T t)
@ -292,8 +297,7 @@ private:
static struct VariableInfo
{
string name;
size_t line;
size_t column;
Token token;
bool isValueType;
}
@ -303,7 +307,7 @@ private:
{
immutable string errorMessage = "Variable " ~ vi.name
~ " is never modified and could have been declared const or immutable.";
addErrorMessage(vi.line, vi.column, "dscanner.suspicious.unmodified", errorMessage);
addErrorMessage(vi.token, KEY, errorMessage);
}
tree = tree[0 .. $ - 1];
}
@ -346,7 +350,8 @@ bool isValueTypeSimple(const Type type) pure nothrow @nogc
// fails
assertAnalyzerWarnings(q{
void foo(){int i = 1;} // [warn]: Variable i is never modified and could have been declared const or immutable.
void foo(){int i = 1;} /+
^ [warn]: Variable i is never modified and could have been declared const or immutable. +/
}, sac);
// pass
@ -381,5 +386,12 @@ bool isValueTypeSimple(const Type type) pure nothrow @nogc
foo(i2);
}
}, sac);
assertAnalyzerWarnings(q{
@("nolint(dscanner.suspicious.unmodified)")
void foo(){
int i = 1;
}
}, sac);
}

View File

@ -20,12 +20,10 @@ abstract class UnusedIdentifierCheck : BaseAnalyzer
alias visit = BaseAnalyzer.visit;
/**
* Params:
* fileName = the name of the file being analyzed
*/
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
re = regex("[\\p{Alphabetic}_][\\w_]*");
}
@ -79,6 +77,13 @@ abstract class UnusedIdentifierCheck : BaseAnalyzer
mixin PartsUseVariables!ThrowExpression;
mixin PartsUseVariables!CastExpression;
override void dynamicDispatch(const ExpressionNode n)
{
interestDepth++;
super.dynamicDispatch(n);
interestDepth--;
}
override void visit(const SwitchStatement switchStatement)
{
if (switchStatement.expression !is null)
@ -92,10 +97,10 @@ abstract class UnusedIdentifierCheck : BaseAnalyzer
override void visit(const WhileStatement whileStatement)
{
if (whileStatement.expression !is null)
if (whileStatement.condition.expression !is null)
{
interestDepth++;
whileStatement.expression.accept(this);
whileStatement.condition.expression.accept(this);
interestDepth--;
}
if (whileStatement.declarationOrStatement !is null)
@ -136,10 +141,10 @@ abstract class UnusedIdentifierCheck : BaseAnalyzer
override void visit(const IfStatement ifStatement)
{
if (ifStatement.expression !is null)
if (ifStatement.condition.expression !is null)
{
interestDepth++;
ifStatement.expression.accept(this);
ifStatement.condition.expression.accept(this);
interestDepth--;
}
if (ifStatement.thenStatement !is null)
@ -338,11 +343,11 @@ abstract class UnusedIdentifierCheck : BaseAnalyzer
protected Tree[] tree;
protected void variableDeclared(string name, size_t line, size_t column, bool isRef)
protected void variableDeclared(string name, Token token, bool isRef)
{
if (inAggregateScope || name.all!(a => a == '_'))
return;
tree[$ - 1].insert(new UnUsed(name, line, column, isRef));
tree[$ - 1].insert(new UnUsed(name, token, isRef));
}
protected void pushScope()
@ -355,8 +360,7 @@ private:
struct UnUsed
{
string name;
size_t line;
size_t column;
Token token;
bool isRef;
bool uncertain;
}
@ -415,15 +419,13 @@ abstract class UnusedStorageCheck : UnusedIdentifierCheck
/**
* Params:
* fileName = the name of the file being analyzed
* sc = the scope
* skipTest = whether tests should be analyzed
* publicType = declaration kind used in error messages, e.g. "Variable"s
* reportType = declaration kind used in error reports, e.g. "unused_variable"
* args = commonly shared analyzer arguments
* publicType = declaration kind used in error messages, e.g. "Variable"s
* reportType = declaration kind used in error reports, e.g. "unused_variable"
*/
this(string fileName, const(Scope)* sc, bool skipTests = false, string publicType = null, string reportType = null)
this(BaseAnalyzerArguments args, string publicType = null, string reportType = null)
{
super(fileName, sc, skipTests);
super(args);
this.publicType = publicType;
this.reportType = reportType;
}
@ -450,8 +452,7 @@ abstract class UnusedStorageCheck : UnusedIdentifierCheck
if (uu.uncertain)
continue;
immutable string errorMessage = publicType ~ ' ' ~ uu.name ~ " is never used.";
addErrorMessage(uu.line, uu.column,
"dscanner.suspicious." ~ reportType, errorMessage);
addErrorMessage(uu.token, "dscanner.suspicious." ~ reportType, errorMessage);
}
}
}

View File

@ -21,9 +21,9 @@ final class UnusedLabelCheck : BaseAnalyzer
mixin AnalyzerInfo!"unused_label_check";
///
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
}
override void visit(const Module mod)
@ -57,16 +57,15 @@ final class UnusedLabelCheck : BaseAnalyzer
override void visit(const LabeledStatement labeledStatement)
{
auto token = &labeledStatement.identifier;
auto token = labeledStatement.identifier;
Label* label = token.text in current;
if (label is null)
{
current[token.text] = Label(token.text, token.line, token.column, false);
current[token.text] = Label(token.text, token, false);
}
else
{
label.line = token.line;
label.column = token.column;
label.token = token;
}
if (labeledStatement.declarationOrStatement !is null)
labeledStatement.declarationOrStatement.accept(this);
@ -116,11 +115,12 @@ final class UnusedLabelCheck : BaseAnalyzer
private:
enum string KEY = "dscanner.suspicious.unused_label";
static struct Label
{
string name;
size_t line;
size_t column;
Token token;
bool used;
}
@ -140,13 +140,13 @@ private:
{
foreach (label; current.byValue())
{
if (label.line == size_t.max || label.column == size_t.max)
if (label.token is Token.init)
{
// TODO: handle unknown labels
}
else if (!label.used)
{
addErrorMessage(label.line, label.column, "dscanner.suspicious.unused_label",
addErrorMessage(label.token, KEY,
"Label \"" ~ label.name ~ "\" is not used.");
}
}
@ -157,7 +157,7 @@ private:
{
Label* entry = name in current;
if (entry is null)
current[name] = Label(name, size_t.max, size_t.max, true);
current[name] = Label(name, Token.init, true);
else
entry.used = true;
}
@ -174,14 +174,16 @@ unittest
int testUnusedLabel()
{
int x = 0;
A: // [warn]: Label "A" is not used.
A: /+
^ [warn]: Label "A" is not used. +/
if (x) goto B;
x++;
B:
goto C;
void foo()
{
C: // [warn]: Label "C" is not used.
C: /+
^ [warn]: Label "C" is not used. +/
return;
}
C:
@ -191,10 +193,12 @@ unittest
D:
return;
}
D: // [warn]: Label "D" is not used.
D: /+
^ [warn]: Label "D" is not used. +/
goto E;
() {
E: // [warn]: Label "E" is not used.
E: /+
^ [warn]: Label "E" is not used. +/
return;
}();
E:
@ -203,9 +207,11 @@ unittest
F:
return;
}();
F: // [warn]: Label "F" is not used.
F: /+
^ [warn]: Label "F" is not used. +/
return x;
G: // [warn]: Label "G" is not used.
G: /+
^ [warn]: Label "G" is not used. +/
}
}c, sac);
@ -221,7 +227,8 @@ unittest
void testAsm()
{
asm { mov RAX,1;}
lbl: // [warn]: Label "lbl" is not used.
lbl: /+
^^^ [warn]: Label "lbl" is not used. +/
}
}c, sac);

View File

@ -23,9 +23,9 @@ final class UnusedParameterCheck : UnusedStorageCheck
* Params:
* fileName = the name of the file being analyzed
*/
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests, "Parameter", "unused_parameter");
super(args, "Parameter", "unused_parameter");
}
override void visit(const Parameter parameter)
@ -41,8 +41,7 @@ final class UnusedParameterCheck : UnusedStorageCheck
immutable bool isPtr = parameter.type && !parameter.type
.typeSuffixes.filter!(a => a.star != tok!"").empty;
variableDeclared(parameter.name.text, parameter.name.line,
parameter.name.column, isRef | isPtr);
variableDeclared(parameter.name.text, parameter.name, isRef | isPtr);
if (parameter.default_ !is null)
{
@ -72,9 +71,11 @@ final class UnusedParameterCheck : UnusedStorageCheck
is(StringTypeOf!R));
}
void inPSC(in int a){} // [warn]: Parameter a is never used.
void inPSC(in int a){} /+
^ [warn]: Parameter a is never used. +/
void doStuff(int a, int b) // [warn]: Parameter b is never used.
void doStuff(int a, int b) /+
^ [warn]: Parameter b is never used. +/
{
return a;
}
@ -95,7 +96,8 @@ final class UnusedParameterCheck : UnusedStorageCheck
{
auto cb1 = delegate(size_t _) {};
cb1(3);
auto cb2 = delegate(size_t a) {}; // [warn]: Parameter a is never used.
auto cb2 = delegate(size_t a) {}; /+
^ [warn]: Parameter a is never used. +/
cb2(3);
}

View File

@ -4,14 +4,15 @@
// http://www.boost.org/LICENSE_1_0.txt)
module dscanner.analysis.unused_result;
import dparse.ast;
import dparse.lexer;
import dscanner.analysis.base;
import dscanner.analysis.mismatched_args : resolveSymbol, IdentVisitor;
import dscanner.analysis.mismatched_args : IdentVisitor, resolveSymbol;
import dscanner.utils;
import dsymbol.scope_;
import dsymbol.symbol;
import dparse.ast, dparse.lexer;
import std.algorithm.searching : canFind;
import std.range: retro;
import std.algorithm : canFind, countUntil;
import std.range : retro;
/**
* Checks for function call statements which call non-void functions.
@ -40,9 +41,9 @@ public:
const(DSymbol)* noreturn_;
///
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests);
super(args);
void_ = sc.getSymbolsByName(internString("void"))[0];
auto symbols = sc.getSymbolsByName(internString("noreturn"));
if (symbols.length > 0)
@ -93,7 +94,13 @@ public:
return;
}
addErrorMessage(decl.expression.line, decl.expression.column, KEY, MSG);
const(Token)[] tokens = fce.unaryExpression
? fce.unaryExpression.tokens
: fce.type
? fce.type.tokens
: fce.tokens;
addErrorMessage(tokens, KEY, MSG);
}
}
@ -128,7 +135,8 @@ unittest
int fun() { return 1; }
void main()
{
fun(); // [warn]: %s
fun(); /+
^^^ [warn]: %s +/
}
}c.format(UnusedResultChecker.MSG), sac);
@ -143,7 +151,8 @@ unittest
alias Bar = Foo;
void main()
{
Bar.get(); // [warn]: %s
Bar.get(); /+
^^^^^^^ [warn]: %s +/
}
}c.format(UnusedResultChecker.MSG), sac);
@ -160,7 +169,8 @@ unittest
void main()
{
int fun() { return 1; }
fun(); // [warn]: %s
fun(); /+
^^^ [warn]: %s +/
}
}c.format(UnusedResultChecker.MSG), sac);

View File

@ -23,22 +23,22 @@ final class UnusedVariableCheck : UnusedStorageCheck
* Params:
* fileName = the name of the file being analyzed
*/
this(string fileName, const(Scope)* sc, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, sc, skipTests, "Variable", "unused_variable");
super(args, "Variable", "unused_variable");
}
override void visit(const VariableDeclaration variableDeclaration)
{
foreach (d; variableDeclaration.declarators)
this.variableDeclared(d.name.text, d.name.line, d.name.column, false);
this.variableDeclared(d.name.text, d.name, false);
variableDeclaration.accept(this);
}
override void visit(const AutoDeclaration autoDeclaration)
{
foreach (t; autoDeclaration.parts.map!(a => a.identifier))
this.variableDeclared(t.text, t.line, t.column, false);
this.variableDeclared(t.text, t, false);
autoDeclaration.accept(this);
}
}
@ -62,7 +62,8 @@ final class UnusedVariableCheck : UnusedStorageCheck
unittest
{
int a; // [warn]: Variable a is never used.
int a; /+
^ [warn]: Variable a is never used. +/
}
// Issue 380
@ -75,7 +76,8 @@ final class UnusedVariableCheck : UnusedStorageCheck
// Issue 380
int otherTemplatedEnum()
{
auto a(T) = T.init; // [warn]: Variable a is never used.
auto a(T) = T.init; /+
^ [warn]: Variable a is never used. +/
return 0;
}
@ -123,6 +125,12 @@ final class UnusedVariableCheck : UnusedStorageCheck
__traits(isPOD);
}
void unitthreaded()
{
auto testVar = foo.sort!myComp;
genVar.should == testVar;
}
}c, sac);
stderr.writeln("Unittest for UnusedVariableCheck passed.");
}

View File

@ -30,9 +30,9 @@ final class UselessAssertCheck : BaseAnalyzer
mixin AnalyzerInfo!"useless_assert_check";
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const AssertExpression ae)
@ -94,7 +94,7 @@ final class UselessAssertCheck : BaseAnalyzer
default:
return;
}
addErrorMessage(ae.line, ae.column, KEY, MESSAGE);
addErrorMessage(unary, KEY, MESSAGE);
}
private:
@ -113,9 +113,12 @@ unittest
assertAnalyzerWarnings(q{
unittest
{
assert(true); // [warn]: %1$s
assert(1); // [warn]: %1$s
assert([10]); // [warn]: %1$s
assert(true); /+
^^^^ [warn]: %1$s +/
assert(1); /+
^ [warn]: %1$s +/
assert([10]); /+
^^^^ [warn]: %1$s +/
assert(false);
assert(0);
assert(0.0L);

View File

@ -5,6 +5,7 @@
module dscanner.analysis.useless_initializer;
import dscanner.analysis.base;
import dscanner.analysis.nolint;
import dscanner.utils : safeAccess;
import containers.dynamicarray;
import containers.hashmap;
@ -33,7 +34,7 @@ final class UselessInitializerChecker : BaseAnalyzer
private:
enum key = "dscanner.useless-initializer";
enum string KEY = "dscanner.useless-initializer";
version(unittest)
{
@ -55,9 +56,9 @@ private:
public:
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
_inStruct.insert(false);
}
@ -92,7 +93,10 @@ public:
override void visit(const(Declaration) decl)
{
_inStruct.insert(decl.structDeclaration !is null);
decl.accept(this);
with (noLint.push(NoLintFactory.fromDeclaration(decl)))
decl.accept(this);
if (_inStruct.length > 1 && _inStruct[$-2] && decl.constructor &&
((decl.constructor.parameters && decl.constructor.parameters.parameters.length == 0) ||
!decl.constructor.parameters))
@ -155,14 +159,18 @@ public:
version(unittest)
{
enum warn = q{addErrorMessage(declarator.name.line,
declarator.name.column, key, msg);};
void warn(const BaseNode range)
{
addErrorMessage(range, KEY, msg);
}
}
else
{
import std.format : format;
enum warn = q{addErrorMessage(declarator.name.line,
declarator.name.column, key, msg.format(declarator.name.text));};
void warn(const BaseNode range)
{
addErrorMessage(range, KEY, msg.format(declarator.name.text));
}
}
// --- Info about the declaration type --- //
@ -203,32 +211,32 @@ public:
case tok!"cent", tok!"ucent":
case tok!"bool":
if (intDefs.canFind(value.text) || value == tok!"false")
mixin(warn);
warn(nvi);
goto default;
default:
// check for BasicType.init
if (ue.primaryExpression.basicType.type == decl.type.type2.builtinType &&
ue.primaryExpression.primary.text == "init" &&
!ue.primaryExpression.expression)
mixin(warn);
warn(nvi);
}
}
else if (isSzInt)
{
if (intDefs.canFind(value.text))
mixin(warn);
warn(nvi);
}
else if (isPtr || isStr)
{
if (str(value.type) == "null")
mixin(warn);
warn(nvi);
}
else if (isArr)
{
if (str(value.type) == "null")
mixin(warn);
warn(nvi);
else if (nvi.arrayInitializer && nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
warn(nvi);
}
}
@ -242,7 +250,7 @@ public:
if (customType.text in _structCanBeInit)
{
if (!_structCanBeInit[customType.text])
mixin(warn);
warn(nvi);
}
}
}
@ -251,7 +259,7 @@ public:
else if (nvi.arrayInitializer && (isArr || isStr))
{
if (nvi.arrayInitializer.arrayMemberInitializations.length == 0)
mixin(warn);
warn(nvi);
}
}
@ -271,25 +279,44 @@ public:
// fails
assertAnalyzerWarnings(q{
struct S {}
ubyte a = 0x0; // [warn]: X
int a = 0; // [warn]: X
ulong a = 0; // [warn]: X
int* a = null; // [warn]: X
Foo* a = null; // [warn]: X
int[] a = null; // [warn]: X
int[] a = []; // [warn]: X
string a = null; // [warn]: X
string a = null; // [warn]: X
wstring a = null; // [warn]: X
dstring a = null; // [warn]: X
size_t a = 0; // [warn]: X
ptrdiff_t a = 0; // [warn]: X
string a = []; // [warn]: X
char[] a = null; // [warn]: X
int a = int.init; // [warn]: X
char a = char.init; // [warn]: X
S s = S.init; // [warn]: X
bool a = false; // [warn]: X
ubyte a = 0x0; /+
^^^ [warn]: X +/
int a = 0; /+
^ [warn]: X +/
ulong a = 0; /+
^ [warn]: X +/
int* a = null; /+
^^^^ [warn]: X +/
Foo* a = null; /+
^^^^ [warn]: X +/
int[] a = null; /+
^^^^ [warn]: X +/
int[] a = []; /+
^^ [warn]: X +/
string a = null; /+
^^^^ [warn]: X +/
string a = null; /+
^^^^ [warn]: X +/
wstring a = null; /+
^^^^ [warn]: X +/
dstring a = null; /+
^^^^ [warn]: X +/
size_t a = 0; /+
^ [warn]: X +/
ptrdiff_t a = 0; /+
^ [warn]: X +/
string a = []; /+
^^ [warn]: X +/
char[] a = null; /+
^^^^ [warn]: X +/
int a = int.init; /+
^^^^^^^^ [warn]: X +/
char a = char.init; /+
^^^^^^^^^ [warn]: X +/
S s = S.init; /+
^^^^^^ [warn]: X +/
bool a = false; /+
^^^^^ [warn]: X +/
}, sac);
// passes
@ -338,6 +365,45 @@ public:
NotKnown nk = NotKnown.init;
}, sac);
// passes
assertAnalyzerWarnings(q{
@("nolint(dscanner.useless-initializer)")
int a = 0;
int a = 0; /+
^ [warn]: X +/
@("nolint(dscanner.useless-initializer)")
int f() {
int a = 0;
}
struct nolint { string s; }
@nolint("dscanner.useless-initializer")
int a = 0;
int a = 0; /+
^ [warn]: X +/
@("nolint(other_check, dscanner.useless-initializer, another_one)")
int a = 0;
@nolint("other_check", "another_one", "dscanner.useless-initializer")
int a = 0;
}, sac);
// passes (disable check at module level)
assertAnalyzerWarnings(q{
@("nolint(dscanner.useless-initializer)")
module my_module;
int a = 0;
int f() {
int a = 0;
}
}, sac);
stderr.writeln("Unittest for UselessInitializerChecker passed.");
}

View File

@ -136,7 +136,7 @@ private:
{
if (call == vm)
{
addErrorMessage(call.line, call.column, KEY, MSG);
addErrorMessage(call, KEY, MSG);
break;
}
}
@ -145,9 +145,9 @@ private:
public:
///
this(string fileName, bool skipTests = false)
this(BaseAnalyzerArguments args)
{
super(fileName, null, skipTests);
super(args);
}
override void visit(const(ClassDeclaration) decl)
@ -306,7 +306,8 @@ unittest
assertAnalyzerWarnings(q{
class Bar
{
this(){foo();} // [warn]: %s
this(){foo();} /+
^^^ [warn]: %s +/
private:
public
void foo(){}
@ -319,8 +320,10 @@ unittest
{
this()
{
foo(); // [warn]: %s
foo(); // [warn]: %s
foo(); /+
^^^ [warn]: %s +/
foo(); /+
^^^ [warn]: %s +/
bar();
}
private: void bar();
@ -334,7 +337,8 @@ unittest
this()
{
foo();
bar(); // [warn]: %s
bar(); /+
^^^ [warn]: %s +/
}
private: public void bar();
public private {void foo(){}}

View File

@ -21,12 +21,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<addExpression operator=\"", str(addExpression.operator), "\">");
output.writeln("<left>");
visit(addExpression.left);
dynamicDispatch(addExpression.left);
output.writeln("</left>");
if (addExpression.right !is null)
{
output.writeln("<right>");
visit(addExpression.right);
dynamicDispatch(addExpression.right);
output.writeln("</right>");
}
output.writeln("</addExpression>");
@ -56,12 +56,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<andAndExpression>");
output.writeln("<left>");
visit(andAndExpression.left);
dynamicDispatch(andAndExpression.left);
output.writeln("</left>");
if (andAndExpression.right !is null)
{
output.writeln("<right>");
visit(andAndExpression.right);
dynamicDispatch(andAndExpression.right);
output.writeln("</right>");
}
output.writeln("</andAndExpression>");
@ -71,12 +71,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<andExpression>");
output.writeln("<left>");
visit(andExpression.left);
dynamicDispatch(andExpression.left);
output.writeln("</left>");
if (andExpression.right !is null)
{
output.writeln("<right>");
visit(andExpression.right);
dynamicDispatch(andExpression.right);
output.writeln("</right>");
}
output.writeln("</andExpression>");
@ -182,13 +182,13 @@ class XMLPrinter : ASTVisitor
if (caseRangeStatement.low !is null)
{
output.writeln("<low>");
visit(caseRangeStatement.low);
dynamicDispatch(caseRangeStatement.low);
output.writeln("</low>");
}
if (caseRangeStatement.high !is null)
{
output.writeln("<high>");
visit(caseRangeStatement.high);
dynamicDispatch(caseRangeStatement.high);
output.writeln("</high>");
}
if (caseRangeStatement.declarationsAndStatements !is null)
@ -286,7 +286,7 @@ class XMLPrinter : ASTVisitor
if (deprecated_.assignExpression !is null)
{
output.writeln("<deprecated>");
visit(deprecated_.assignExpression);
dynamicDispatch(deprecated_.assignExpression);
output.writeln("</deprecated>");
}
else
@ -311,7 +311,7 @@ class XMLPrinter : ASTVisitor
visit(enumMember.type);
output.write("<name>", enumMember.name.text, "</name>");
if (enumMember.assignExpression !is null)
visit(enumMember.assignExpression);
dynamicDispatch(enumMember.assignExpression);
output.writeln("</anonymousEnumMember>");
}
@ -327,10 +327,10 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<equalExpression operator=\"", str(equalExpression.operator), "\">");
output.writeln("<left>");
visit(equalExpression.left);
dynamicDispatch(equalExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(equalExpression.right);
dynamicDispatch(equalExpression.right);
output.writeln("</right>");
output.writeln("</equalExpression>");
}
@ -447,10 +447,10 @@ class XMLPrinter : ASTVisitor
else
output.writeln("<identityExpression operator=\"is\">");
output.writeln("<left>");
visit(identityExpression.left);
dynamicDispatch(identityExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(identityExpression.right);
dynamicDispatch(identityExpression.right);
output.writeln("</right>");
output.writeln("</identityExpression>");
}
@ -460,15 +460,15 @@ class XMLPrinter : ASTVisitor
output.writeln("<ifStatement>");
output.writeln("<condition>");
if (ifStatement.identifier.type != tok!"")
if (ifStatement.condition.identifier.type != tok!"")
{
if (ifStatement.type is null)
if (ifStatement.condition.type is null)
output.writeln("<auto/>");
else
visit(ifStatement.type);
visit(ifStatement.identifier);
visit(ifStatement.condition.type);
visit(ifStatement.condition.identifier);
}
ifStatement.expression.accept(this);
ifStatement.condition.expression.accept(this);
output.writeln("</condition>");
output.writeln("<then>");
@ -500,10 +500,10 @@ class XMLPrinter : ASTVisitor
else
output.writeln("<inExpression operator=\"in\">");
output.writeln("<left>");
visit(inExpression.left);
dynamicDispatch(inExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(inExpression.right);
dynamicDispatch(inExpression.right);
output.writeln("</right>");
output.writeln("</inExpression>");
}
@ -572,10 +572,10 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<keyValuePair>");
output.writeln("<key>");
visit(keyValuePair.key);
dynamicDispatch(keyValuePair.key);
output.writeln("</key>");
output.writeln("<value>");
visit(keyValuePair.value);
dynamicDispatch(keyValuePair.value);
output.writeln("</value>");
output.writeln("</keyValuePair>");
}
@ -635,12 +635,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<mulExpression operator=\"", str(mulExpression.operator), "\">");
output.writeln("<left>");
visit(mulExpression.left);
dynamicDispatch(mulExpression.left);
output.writeln("</left>");
if (mulExpression.right !is null)
{
output.writeln("<right>");
visit(mulExpression.right);
dynamicDispatch(mulExpression.right);
output.writeln("</right>");
}
output.writeln("</mulExpression>");
@ -650,12 +650,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<orOrExpression>");
output.writeln("<left>");
visit(orOrExpression.left);
dynamicDispatch(orOrExpression.left);
output.writeln("</left>");
if (orOrExpression.right !is null)
{
output.writeln("<right>");
visit(orOrExpression.right);
dynamicDispatch(orOrExpression.right);
output.writeln("</right>");
}
output.writeln("</orOrExpression>");
@ -686,12 +686,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<powExpression>");
output.writeln("<left>");
visit(powExpression.left);
dynamicDispatch(powExpression.left);
output.writeln("</left>");
if (powExpression.right !is null)
{
output.writeln("<right>");
visit(powExpression.right);
dynamicDispatch(powExpression.right);
output.writeln("</right>");
}
output.writeln("</powExpression>");
@ -702,10 +702,10 @@ class XMLPrinter : ASTVisitor
output.writeln("<relExpression operator=\"",
xmlAttributeEscape(str(relExpression.operator)), "\">");
output.writeln("<left>");
visit(relExpression.left);
dynamicDispatch(relExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(relExpression.right);
dynamicDispatch(relExpression.right);
output.writeln("</right>");
output.writeln("</relExpression>");
}
@ -727,10 +727,10 @@ class XMLPrinter : ASTVisitor
output.writeln("<shiftExpression operator=\"",
xmlAttributeEscape(str(shiftExpression.operator)), "\">");
output.writeln("<left>");
visit(shiftExpression.left);
dynamicDispatch(shiftExpression.left);
output.writeln("</left>");
output.writeln("<right>");
visit(shiftExpression.right);
dynamicDispatch(shiftExpression.right);
output.writeln("</right>");
output.writeln("</shiftExpression>");
}
@ -763,7 +763,7 @@ class XMLPrinter : ASTVisitor
if (templateAliasParameter.colonExpression !is null)
{
output.writeln("<specialization>");
visit(templateAliasParameter.colonExpression);
dynamicDispatch(templateAliasParameter.colonExpression);
output.writeln("</specialization>");
}
else if (templateAliasParameter.colonType !is null)
@ -776,7 +776,7 @@ class XMLPrinter : ASTVisitor
if (templateAliasParameter.assignExpression !is null)
{
output.writeln("<default>");
visit(templateAliasParameter.assignExpression);
dynamicDispatch(templateAliasParameter.assignExpression);
output.writeln("</default>");
}
else if (templateAliasParameter.assignType !is null)
@ -921,14 +921,14 @@ class XMLPrinter : ASTVisitor
if (typeSuffix.high !is null)
{
output.writeln("<low>");
visit(typeSuffix.low);
dynamicDispatch(typeSuffix.low);
output.writeln("</low>");
output.writeln("<high>");
visit(typeSuffix.high);
dynamicDispatch(typeSuffix.high);
output.writeln("</high>");
}
else
visit(typeSuffix.low);
dynamicDispatch(typeSuffix.low);
output.writeln("</typeSuffix>");
}
}
@ -1000,12 +1000,12 @@ class XMLPrinter : ASTVisitor
{
output.writeln("<xorExpression>");
output.writeln("<left>");
visit(xorExpression.left);
dynamicDispatch(xorExpression.left);
output.writeln("</left>");
if (xorExpression.right !is null)
{
output.writeln("<right>");
visit(xorExpression.right);
dynamicDispatch(xorExpression.right);
output.writeln("</right>");
}
output.writeln("</xorExpression>");
@ -1017,23 +1017,34 @@ class XMLPrinter : ASTVisitor
if (index.high)
{
output.writeln("<low>");
visit(index.low);
dynamicDispatch(index.low);
output.writeln("</low>");
output.writeln("<high>");
visit(index.high);
dynamicDispatch(index.high);
output.writeln("</high>");
}
else
visit(index.low);
dynamicDispatch(index.low);
output.writeln("</index>");
}
override void visit(const NamedArgument arg)
{
if (arg.name.text.length)
output.writeln("<argument named=\"", arg.name.text, "\">");
else
output.writeln("<argument>");
dynamicDispatch(arg.assignExpression);
output.writeln("</argument>");
}
// dfmt off
override void visit(const AliasInitializer aliasInitializer) { mixin (tagAndAccept!"aliasInitializer"); }
override void visit(const AliasThisDeclaration aliasThisDeclaration) { mixin (tagAndAccept!"aliasThisDeclaration"); }
override void visit(const AnonymousEnumDeclaration anonymousEnumDeclaration) { mixin (tagAndAccept!"anonymousEnumDeclaration"); }
override void visit(const ArgumentList argumentList) { mixin (tagAndAccept!"argumentList"); }
override void visit(const NamedArgumentList argumentList) { mixin (tagAndAccept!"argumentList"); }
override void visit(const Arguments arguments) { mixin (tagAndAccept!"arguments"); }
override void visit(const ArrayInitializer arrayInitializer) { mixin (tagAndAccept!"arrayInitializer"); }
override void visit(const ArrayLiteral arrayLiteral) { mixin (tagAndAccept!"arrayLiteral"); }

View File

@ -10,8 +10,10 @@ import std.array;
import dparse.lexer;
// http://ethanschoonover.com/solarized
void highlight(R)(ref R tokens, string fileName)
void highlight(R)(ref R tokens, string fileName, string themeName)
{
immutable(Theme)* theme = getTheme(themeName);
stdout.writeln(q"[
<!DOCTYPE html>
<html>
@ -20,17 +22,19 @@ void highlight(R)(ref R tokens, string fileName)
stdout.writeln("<title>", fileName, "</title>");
stdout.writeln(q"[</head>
<body>
<style type="text/css">
html { background-color: #fdf6e3; color: #002b36; }
.kwrd { color: #b58900; font-weight: bold; }
.com { color: #93a1a1; font-style: italic; }
.num { color: #dc322f; font-weight: bold; }
.str { color: #2aa198; font-style: italic; }
.op { color: #586e75; font-weight: bold; }
.type { color: #268bd2; font-weight: bold; }
.cons { color: #859900; font-weight: bold; }
<style type="text/css">]");
stdout.writefln("
html { background-color: %s; color: %s; }
.kwrd { color: %s; font-weight: bold; }
.com { color: %s; font-style: italic; }
.num { color: %s; font-weight: bold; }
.str { color: %s; font-style: italic; }
.op { color: %s; font-weight: bold; }
.type { color: %s; font-weight: bold; }
.cons { color: %s; font-weight: bold; }
</style>
<pre>]");
<pre>", theme.bg, theme.fg, theme.kwrd, theme.com, theme.num, theme.str,
theme.op, theme.type, theme.cons);
while (!tokens.empty)
{
@ -76,3 +80,37 @@ void writeSpan(string cssClass, string value)
stdout.write(`<span class="`, cssClass, `">`, value.replace("&",
"&amp;").replace("<", "&lt;"), `</span>`);
}
struct Theme
{
string bg;
string fg;
string kwrd;
string com;
string num;
string str;
string op;
string type;
string cons;
}
immutable(Theme)* getTheme(string themeName)
{
immutable Theme[string] themes = [
"solarized": Theme("#fdf6e3", "#002b36", "#b58900", "#93a1a1", "#dc322f", "#2aa198", "#586e75",
"#268bd2", "#859900"),
"solarized-dark": Theme("#002b36", "#fdf6e3", "#b58900", "#586e75", "#dc322f", "#2aa198",
"#93a1a1", "#268bd2", "#859900"),
"gruvbox": Theme("#fbf1c7", "#282828", "#b57614", "#a89984", "#9d0006", "#427b58",
"#504945", "#076678", "#79740e"),
"gruvbox-dark": Theme("#282828", "#fbf1c7", "#d79921", "#7c6f64",
"#cc241d", "#689d6a", "#a89984", "#458588", "#98971a")
];
immutable(Theme)* theme = themeName in themes;
// Default theme
if (theme is null)
theme = &themes["solarized"];
return theme;
}

View File

@ -5,20 +5,21 @@
module dscanner.main;
import std.algorithm;
import std.array;
import std.conv;
import std.file;
import std.getopt;
import std.path;
import std.stdio;
import std.range;
import std.experimental.lexer;
import std.typecons : scoped;
import std.functional : toDelegate;
import dparse.lexer;
import dparse.parser;
import dparse.rollback_allocator;
import std.algorithm;
import std.array;
import std.conv;
import std.experimental.lexer;
import std.file;
import std.functional : toDelegate;
import std.getopt;
import std.path;
import std.range;
import std.stdio;
import std.string : chomp, splitLines;
import std.typecons : scoped;
import dscanner.highlighter;
import dscanner.stats;
@ -44,6 +45,7 @@ version (unittest)
else
int main(string[] args)
{
bool autofix;
bool sloc;
bool highlight;
bool ctags;
@ -62,21 +64,30 @@ else
bool defaultConfig;
bool report;
bool skipTests;
bool applySingleFixes;
string theme;
string resolveMessage;
string reportFormat;
string reportFile;
string symbolName;
string configLocation;
string[] importPaths;
string[] excludePaths;
bool printVersion;
bool explore;
bool verbose;
string errorFormat;
if (args.length == 2 && args[1].startsWith("@"))
args = args[0] ~ readText(args[1][1 .. $]).chomp.splitLines;
try
{
// dfmt off
getopt(args, std.getopt.config.caseSensitive,
"sloc|l", &sloc,
"highlight", &highlight,
"theme", &theme,
"ctags|c", &ctags,
"help|h", &help,
"etags|e", &etags,
@ -95,12 +106,17 @@ else
"report", &report,
"reportFormat", &reportFormat,
"reportFile", &reportFile,
"resolveMessage", &resolveMessage,
"applySingle", &applySingleFixes,
"I", &importPaths,
"exclude", &excludePaths,
"version", &printVersion,
"muffinButton", &muffin,
"explore", &explore,
"skipTests", &skipTests,
"errorFormat|f", &errorFormat);
"errorFormat|f", &errorFormat,
"verbose|v", &verbose
);
//dfmt on
}
catch (ConvException e)
@ -114,6 +130,21 @@ else
return 1;
}
{
static if (__VERSION__ >= 2_101)
import std.logger : sharedLog, LogLevel;
else
import std.experimental.logger : globalLogLevel, LogLevel;
// we don't use std.logger, but dsymbol does, so we surpress all
// messages that aren't errors from it by default
// users can use verbose to enable all logs (this will log things like
// dsymbol couldn't find some modules due to wrong import paths)
static if (__VERSION__ >= 2_101)
(cast()sharedLog).logLevel = verbose ? LogLevel.all : LogLevel.error;
else
globalLogLevel = verbose ? LogLevel.all : LogLevel.error;
}
if (muffin)
{
stdout.writeln(` ___________
@ -147,11 +178,57 @@ else
return 0;
}
if (args.length > 1)
{
switch (args[1])
{
case "lint":
args = args[0] ~ args[2 .. $];
styleCheck = true;
if (!errorFormat.length)
errorFormat = "pretty";
break;
case "fix":
args = args[0] ~ args[2 .. $];
autofix = true;
if (!errorFormat.length)
errorFormat = "pretty";
break;
default:
break;
}
}
auto expandedArgs = () {
auto expanded = expandArgs(args);
if (excludePaths.length)
{
string[] newArgs = [expanded[0]];
foreach(arg; args[1 .. $])
{
if(!excludePaths.map!(p => arg.isSubpathOf(p))
.fold!((a, b) => a || b))
newArgs ~= arg;
}
return newArgs;
}
else
return expanded;
}();
if (!errorFormat.length)
errorFormat = defaultErrorFormat;
else if (auto errorFormatSuppl = errorFormat in errorFormatMap)
errorFormat = (*errorFormatSuppl)
// support some basic formatting things so it's easier for the user to type these
.replace("\\x1B", "\x1B")
.replace("\\033", "\x1B")
.replace("\\r", "\r")
.replace("\\n", "\n")
.replace("\\t", "\t");
const(string[]) absImportPaths = importPaths.map!(a => a.absolutePath()
.buildNormalizedPath()).array();
const(string[]) absImportPaths = importPaths.map!absoluteNormalizedPath.array;
ModuleCache moduleCache;
@ -161,9 +238,11 @@ else
if (reportFormat.length || reportFile.length)
report = true;
immutable optionCount = count!"a"([sloc, highlight, ctags, tokenCount, syntaxCheck, ast, imports,
outline, tokenDump, styleCheck, defaultConfig, report,
symbolName !is null, etags, etagsAll, recursiveImports]);
immutable optionCount = count!"a"([sloc, highlight, ctags, tokenCount,
syntaxCheck, ast, imports, outline, tokenDump, styleCheck,
defaultConfig, report, autofix, resolveMessage.length,
symbolName !is null, etags, etagsAll, recursiveImports,
]);
if (optionCount > 1)
{
stderr.writeln("Too many options specified");
@ -199,7 +278,7 @@ else
if (highlight)
{
auto tokens = byToken(bytes, config, &cache);
dscanner.highlighter.highlight(tokens, args.length == 1 ? "stdin" : args[1]);
dscanner.highlighter.highlight(tokens, args.length == 1 ? "stdin" : args[1], theme);
return 0;
}
else if (tokenDump)
@ -224,17 +303,17 @@ else
}
else if (symbolName !is null)
{
stdout.findDeclarationOf(symbolName, expandArgs(args));
stdout.findDeclarationOf(symbolName, expandedArgs);
}
else if (ctags)
{
stdout.printCtags(expandArgs(args));
stdout.printCtags(expandedArgs);
}
else if (etags || etagsAll)
{
stdout.printEtags(etagsAll, expandArgs(args));
stdout.printEtags(etagsAll, expandedArgs);
}
else if (styleCheck)
else if (styleCheck || autofix || resolveMessage.length)
{
StaticAnalysisConfig config = defaultStaticAnalysisConfig();
string s = configLocation is null ? getConfigurationLocation() : configLocation;
@ -242,7 +321,17 @@ else
readINIFile(config, s);
if (skipTests)
config.enabled2SkipTests;
if (report)
if (autofix)
{
return .autofix(expandedArgs, config, errorFormat, cache, moduleCache, applySingleFixes) ? 1 : 0;
}
else if (resolveMessage.length)
{
listAutofixes(config, resolveMessage, usingStdin, usingStdin ? "stdin" : args[1], &cache, moduleCache);
return 0;
}
else if (report)
{
switch (reportFormat)
{
@ -251,19 +340,19 @@ else
goto case;
case "":
case "dscanner":
generateReport(expandArgs(args), config, cache, moduleCache, reportFile);
generateReport(expandedArgs, config, cache, moduleCache, reportFile);
break;
case "sonarQubeGenericIssueData":
generateSonarQubeGenericIssueDataReport(expandArgs(args), config, cache, moduleCache, reportFile);
generateSonarQubeGenericIssueDataReport(expandedArgs, config, cache, moduleCache, reportFile);
break;
}
}
else
return analyze(expandArgs(args), config, errorFormat, cache, moduleCache, true) ? 1 : 0;
return analyze(expandedArgs, config, errorFormat, cache, moduleCache, true) ? 1 : 0;
}
else if (syntaxCheck)
{
return .syntaxCheck(usingStdin ? ["stdin"] : expandArgs(args), errorFormat, cache, moduleCache) ? 1 : 0;
return .syntaxCheck(usingStdin ? ["stdin"] : expandedArgs, errorFormat, cache, moduleCache) ? 1 : 0;
}
else
{
@ -282,7 +371,7 @@ else
else
{
ulong count;
foreach (f; expandArgs(args))
foreach (f; expandedArgs)
{
LexerConfig config;
@ -329,8 +418,18 @@ else
void printHelp(string programName)
{
stderr.writefln(`
Usage: %s <options>
stdout.writefln(`
Usage: %1$s <options>
Human-readable output:
%1$s lint <options> <files...>
Interactively fixing issues
%1$s fix [--applySingle] <files...>
Parsable outputs:
%1$s -S <options> <files...>
%1$s --report <options> <files...>
Options:
--help, -h
@ -370,6 +469,9 @@ Options:
modules. This option can be passed multiple times to specify multiple
directories.
--exclude <file | directory>..., <file | directory>
Specify files or directories that will be ignored by D-Scanner.
--syntaxCheck <file>, -s <file>
Lexes and parses sourceFile, printing the line and column number of
any syntax errors to stdout. One error or warning is printed per line,
@ -387,8 +489,30 @@ Options:
--errorFormat|f <pattern>
Format errors produced by the style/syntax checkers. The default
value for the pattern is: "%2$s".
Supported placeholders are: {filepath}, {line}, {column}, {type},
{message}, and {name}.
Supported placeholders are:
- {filepath}: file path, usually relative to CWD
- {line}: start line number, 1-based
- {endLine}: end line number, 1-based, inclusive
- {column}: start column on start line, 1-based, in bytes
- {endColumn}: end column on end line, 1-based, in bytes, exclusive
- {startIndex}: start file byte offset, 0-based
- {endIndex}: end file byte offset, 0-based
- {type}: "error" or "warn", uppercase variants: {Type}, {TYPE},
- {type2}: "error" or "warning", uppercase variants: {Type2}, {TYPE2}
- {message}: human readable message such as "Variable c is never used."
- {name}: D-Scanner check name such as "unused_variable_check"
- {context}: "\n<source code>\n ^^^^^ here"
- {supplemental}: for supplemental messages, each one formatted using
this same format string, tab indented, type = "hint".
For compatibility with other tools, the following strings may be
specified as shorthand aliases:
%3$(-f %1$s -> %2$s` ~ '\n' ~ ` %)
When calling "%1$s lint" for human readable output, "pretty"
is used by default.
--ctags <file | directory>..., -c <file | directory>...
Generates ctags information from the given source code file. Note that
@ -433,9 +557,13 @@ Options:
--skipTests
Does not analyze code in unittests. Only works if --styleCheck
is specified.`,
is specified.
programName, defaultErrorFormat);
--applySingle
when running "dscanner fix", automatically apply all fixes that have
only one auto-fix.`,
programName, defaultErrorFormat, errorFormatMap);
}
private void doNothing(string, size_t, size_t, string, bool)
@ -446,6 +574,9 @@ private enum CONFIG_FILE_NAME = "dscanner.ini";
version (linux) version = useXDG;
version (BSD) version = useXDG;
version (FreeBSD) version = useXDG;
version (OpenBSD) version = useXDG;
version (NetBSD) version = useXDG;
version (DragonflyBSD) version = useXDG;
version (OSX) version = useXDG;
/**

View File

@ -55,14 +55,56 @@ class DScannerJsonReporter
private static JSONValue toJson(Issue issue)
{
import std.sumtype : match;
import dscanner.analysis.base : AutoFix;
// dfmt off
JSONValue js = JSONValue([
"key": JSONValue(issue.message.key),
"fileName": JSONValue(issue.message.fileName),
"line": JSONValue(issue.message.line),
"column": JSONValue(issue.message.column),
"line": JSONValue(issue.message.startLine),
"column": JSONValue(issue.message.startColumn),
"index": JSONValue(issue.message.startIndex),
"endLine": JSONValue(issue.message.endLine),
"endColumn": JSONValue(issue.message.endColumn),
"endIndex": JSONValue(issue.message.endIndex),
"message": JSONValue(issue.message.message),
"type": JSONValue(issue.type)
"type": JSONValue(issue.type),
"supplemental": JSONValue(
issue.message.supplemental.map!(a =>
JSONValue([
"fileName": JSONValue(a.fileName),
"line": JSONValue(a.startLine),
"column": JSONValue(a.startColumn),
"index": JSONValue(a.startIndex),
"endLine": JSONValue(a.endLine),
"endColumn": JSONValue(a.endColumn),
"endIndex": JSONValue(a.endIndex),
"message": JSONValue(a.message),
])
).array
),
"autofixes": JSONValue(
issue.message.autofixes.map!(a =>
JSONValue([
"name": JSONValue(a.name),
"replacements": a.replacements.match!(
(const AutoFix.CodeReplacement[] replacements) => JSONValue(
replacements.map!(r => JSONValue([
"range": JSONValue([
JSONValue(r.range[0]),
JSONValue(r.range[1])
]),
"newText": JSONValue(r.newText)
])).array
),
(const AutoFix.ResolveContext _) => JSONValue(
"resolvable"
)
)
])
).array
)
]);
// dfmt on
@ -129,10 +171,10 @@ class SonarQubeGenericIssueDataReporter
struct TextRange
{
// SonarQube Generic Issue only allows specifying start line only
// or the complete range, for which start and end has to differ
// D-Scanner does not provide the complete range info
long startLine;
long endLine;
long startColumn;
long endColumn;
}
private Appender!(Issue[]) _issues;
@ -160,6 +202,20 @@ class SonarQubeGenericIssueDataReporter
return result.toPrettyString();
}
private static JSONValue toJson(Location location)
{
return JSONValue([
"message": JSONValue(location.message),
"filePath": JSONValue(location.filePath),
"textRange": JSONValue([
"startLine": JSONValue(location.textRange.startLine),
"endLine": JSONValue(location.textRange.endLine),
"startColumn": JSONValue(location.textRange.startColumn),
"endColumn": JSONValue(location.textRange.endColumn)
]),
]);
}
private static JSONValue toJson(Issue issue)
{
// dfmt off
@ -168,13 +224,8 @@ class SonarQubeGenericIssueDataReporter
"ruleId": JSONValue(issue.ruleId),
"severity": JSONValue(issue.severity),
"type": JSONValue(issue.type),
"primaryLocation": JSONValue([
"message": JSONValue(issue.primaryLocation.message),
"filePath": JSONValue(issue.primaryLocation.filePath),
"textRange": JSONValue([
"startLine": JSONValue(issue.primaryLocation.textRange.startLine)
]),
]),
"primaryLocation": toJson(issue.primaryLocation),
"secondaryLocations": JSONValue(issue.secondaryLocations.map!toJson.array),
]);
// dfmt on
}
@ -189,18 +240,20 @@ class SonarQubeGenericIssueDataReporter
// dfmt off
Issue issue = {
engineId: "dscanner",
ruleId : message.key,
severity : (isError) ? Severity.blocker : getSeverity(message.key),
type : getType(message.key),
primaryLocation : getPrimaryLocation(message)
ruleId: message.key,
severity: (isError) ? Severity.blocker : getSeverity(message.key),
type: getType(message.key),
primaryLocation: getLocation(message.diagnostic),
secondaryLocations: message.supplemental.map!getLocation.array
};
// dfmt on
return issue;
}
private static Location getPrimaryLocation(Message message)
private static Location getLocation(Message.Diagnostic diag)
{
return Location(message.message, message.fileName, TextRange(message.line));
return Location(diag.message, diag.fileName,
TextRange(diag.startLine, diag.endLine, diag.startColumn, diag.endColumn));
}
private static string getSeverity(string key)

View File

@ -6,6 +6,7 @@ import std.conv : to;
import std.encoding : BOM, BOMSeq, EncodingException, getBOM;
import std.format : format;
import std.file : exists, read;
import std.path: isValidPath;
private void processBOM(ref ubyte[] sourceCode, string fname)
{
@ -73,13 +74,26 @@ ubyte[] readFile(string fileName)
stderr.writefln("%s does not exist", fileName);
return [];
}
File f = File(fileName);
ubyte[] sourceCode;
sourceCode = cast(ubyte[]) fileName.read();
sourceCode.processBOM(fileName);
return sourceCode;
}
void writeFileSafe(string filename, scope const(ubyte)[] content)
{
import std.file : copy, PreserveAttributes, remove, write;
import std.path : baseName, buildPath, dirName;
string tempName = buildPath(filename.dirName, "." ~ filename.baseName ~ "~");
// FIXME: we are removing the optional BOM here
copy(filename, tempName, PreserveAttributes.yes);
write(filename, content);
remove(tempName);
}
string[] expandArgs(string[] args)
{
import std.file : isFile, FileException, dirEntries, SpanMode;
@ -115,12 +129,63 @@ string[] expandArgs(string[] args)
return rVal;
}
package string absoluteNormalizedPath(in string path)
{
import std.path: absolutePath, buildNormalizedPath;
return path.absolutePath().buildNormalizedPath();
}
private bool areSamePath(in string path1, in string path2)
in(path1.isValidPath && path2.isValidPath)
{
return path1.absoluteNormalizedPath() == path2.absoluteNormalizedPath();
}
unittest
{
assert(areSamePath("/abc/efg", "/abc/efg"));
assert(areSamePath("/abc/../abc/efg", "/abc/efg"));
assert(!areSamePath("/abc/../abc/../efg", "/abc/efg"));
}
package bool isSubpathOf(in string potentialSubPath, in string base)
in(base.isValidPath && potentialSubPath.isValidPath)
{
import std.path: isValidPath, relativePath;
import std.algorithm: canFind;
if(areSamePath(base, potentialSubPath))
return true;
const relative = relativePath(
potentialSubPath.absoluteNormalizedPath(),
base.absoluteNormalizedPath()
);
// No '..' in the relative paths means that potentialSubPath
// is actually a descendant of base
return !relative.canFind("..");
}
unittest
{
const base = "/abc/efg";
assert("/abc/efg/".isSubpathOf(base));
assert("/abc/efg/hij/".isSubpathOf(base));
assert("/abc/efg/hij/../kel".isSubpathOf(base));
assert(!"/abc/kel".isSubpathOf(base));
assert(!"/abc/efg/../kel".isSubpathOf(base));
}
/**
* Allows to build access chains of class members as done with the $(D ?.) operator
* in other languages. In the chain, any $(D null) member that is a class instance
* or that returns one, has for effect to shortcut the complete evaluation.
*
* This function is copied from https://github.com/BBasile/iz to avoid a new submodule.
* This function is copied from
* https://gitlab.com/basile.b/iz/-/blob/18f5c1e78a89edae9f7bd9c2d8e7e0c152f56696/import/iz/sugar.d#L1543
* to avoid adding additional dependencies.
* Any change made to this copy should also be applied to the origin.
*
* Params:

113
tests/dscanner.ini Normal file
View File

@ -0,0 +1,113 @@
; Configure which static analysis checks are enabled
[analysis.config.StaticAnalysisConfig]
; Check variable, class, struct, interface, union, and function names against the Phobos style guide
style_check="enabled"
; Check for array literals that cause unnecessary allocation
enum_array_literal_check="enabled"
; Check for poor exception handling practices
exception_check="enabled"
; Check for use of the deprecated 'delete' keyword
delete_check="enabled"
; Check for use of the deprecated floating point operators
float_operator_check="enabled"
; Check number literals for readability
number_style_check="enabled"
; Checks that opEquals, opCmp, toHash, and toString are either const, immutable, or inout.
object_const_check="enabled"
; Checks for .. expressions where the left side is larger than the right.
backwards_range_check="enabled"
; Checks for if statements whose 'then' block is the same as the 'else' block
if_else_same_check="enabled"
; Checks for some problems with constructors
constructor_check="enabled"
; Checks for unused variables
unused_variable_check="enabled"
; Checks for unused labels
unused_label_check="enabled"
; Checks for unused function parameters
unused_parameter_check="enabled"
; Checks for duplicate attributes
duplicate_attribute="enabled"
; Checks that opEquals and toHash are both defined or neither are defined
opequals_tohash_check="enabled"
; Checks for subtraction from .length properties
length_subtraction_check="enabled"
; Checks for methods or properties whose names conflict with built-in properties
builtin_property_names_check="enabled"
; Checks for confusing code in inline asm statements
asm_style_check="enabled"
; Checks for confusing logical operator precedence
logical_precedence_check="enabled"
; Checks for undocumented public declarations
undocumented_declaration_check="disabled"
; Checks for poor placement of function attributes
function_attribute_check="enabled"
; Checks for use of the comma operator
comma_expression_check="enabled"
; Checks for local imports that are too broad. Only accurate when checking code used with D versions older than 2.071.0
local_import_check="enabled"
; Checks for variables that could be declared immutable
could_be_immutable_check="enabled"
; Checks for redundant expressions in if statements
redundant_if_check="enabled"
; Checks for redundant parenthesis
redundant_parens_check="enabled"
; Checks for mismatched argument and parameter names
mismatched_args_check="enabled"
; Checks for labels with the same name as variables
label_var_same_name_check="enabled"
; Checks for lines longer than `max_line_length` characters
long_line_check="enabled"
; Checks for assignment to auto-ref function parameters
auto_ref_assignment_check="enabled"
; Checks for incorrect infinite range definitions
incorrect_infinite_range_check="enabled"
; Checks for asserts that are always true
useless_assert_check="enabled"
; Check for uses of the old-style alias syntax
alias_syntax_check="enabled"
; Checks for else if that should be else static if
static_if_else_check="enabled"
; Check for unclear lambda syntax
lambda_return_check="enabled"
; Check for auto function without return statement
auto_function_check="enabled"
; Check for sortedness of imports
imports_sortedness="enabled"
; Check for explicitly annotated unittests
explicitly_annotated_unittests="enabled"
; Check for properly documented public functions (Returns, Params)
properly_documented_public_functions="enabled"
; Check for useless usage of the final attribute
final_attribute_check="enabled"
; Check for virtual calls in the class constructors
vcall_in_ctor="enabled"
; Check for useless user defined initializers
useless_initializer="enabled"
; Check allman brace style
allman_braces_check="enabled"
; Check for redundant attributes
redundant_attributes_check="enabled"
; Check public declarations without a documented unittest
has_public_example="enabled"
; Check for asserts without an explanatory message
assert_without_msg="enabled"
; Check indent of if constraints
if_constraints_indent="enabled"
; Check for @trusted applied to a bigger scope than a single function
trust_too_much="enabled"
; Check for redundant storage classes on variable declarations
redundant_storage_classes="enabled"
; Check for unused function return values
unused_result="enabled"
; Enable cyclomatic complexity check
cyclomatic_complexity="enabled"
; Check for function bodies on discord functions
body_on_disabled_func_check="enabled"
; Formatting brace style for automatic fixes (allman, otbs, stroustrup, knr)
brace_style="allman"
; Formatting indentation style for automatic fixes (tabs, spaces)
indentation_style="tab"
; Formatting line ending character (lf, cr, crlf)
eol_style="lf"

82
tests/it.sh Executable file
View File

@ -0,0 +1,82 @@
#!/bin/bash
set -eu -o pipefail
function section {
e=$'\e'
if [ ! -z "${GITHUB_ACTION:-}" ]; then
echo "::endgroup::"
echo "::group::$@"
else
echo "$e[1m$@$e[m"
fi
}
function error {
echo $'\e[31;1mTests have failed.\e[m'
exit 1
}
function cleanup {
if [ ! -z "${GITHUB_ACTION:-}" ]; then
echo "::endgroup::"
fi
}
DSCANNER_DIR="$(dirname -- $( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ))"
if [ ! -z "${GITHUB_ACTION:-}" ]; then
echo "::group::Building d-scanner"
fi
trap cleanup EXIT
trap error ERR
if [ -z "${CI:-}" ]; then
dub build --root="$DSCANNER_DIR"
fi
cd "$DSCANNER_DIR/tests"
# IDE APIs
# --------
# checking that reporting format stays consistent or only gets extended
diff <(../bin/dscanner --report it/autofix_ide/source_autofix.d | jq -S .) <(jq -S . it/autofix_ide/source_autofix.report.json)
diff <(../bin/dscanner --resolveMessage b16 it/autofix_ide/source_autofix.d | jq -S .) <(jq -S . it/autofix_ide/source_autofix.autofix.json)
# CLI tests
# ---------
# check that `dscanner fix` works as expected
section '1. test no changes if EOFing'
cp -v it/autofix_cli/source.d it/autofix_cli/test.d
printf "" | ../bin/dscanner fix it/autofix_cli/test.d
diff it/autofix_cli/test.d it/autofix_cli/source.d
section '2. test no changes for simple enter pressing'
cp -v it/autofix_cli/source.d it/autofix_cli/test.d
printf "\n" | ../bin/dscanner fix it/autofix_cli/test.d
diff it/autofix_cli/test.d it/autofix_cli/source.d
section '2.1. test no changes entering 0'
cp -v it/autofix_cli/source.d it/autofix_cli/test.d
printf "0\n" | ../bin/dscanner fix it/autofix_cli/test.d
diff it/autofix_cli/test.d it/autofix_cli/source.d
section '3. test change applies automatically with --applySingle'
cp -v it/autofix_cli/source.d it/autofix_cli/test.d
../bin/dscanner fix --applySingle it/autofix_cli/test.d | grep -F 'Writing changes to it/autofix_cli/test.d'
diff it/autofix_cli/test.d it/autofix_cli/fixed.d
section '4. test change apply when entering "1"'
cp -v it/autofix_cli/source.d it/autofix_cli/test.d
printf "1\n" | ../bin/dscanner fix it/autofix_cli/test.d | grep -F 'Writing changes to it/autofix_cli/test.d'
diff it/autofix_cli/test.d it/autofix_cli/fixed.d
section '5. test invalid selection reasks what to apply'
cp -v it/autofix_cli/source.d it/autofix_cli/test.d
printf "2\n-1\n1000\na\n1\n" | ../bin/dscanner fix it/autofix_cli/test.d | grep -F 'Writing changes to it/autofix_cli/test.d'
diff it/autofix_cli/test.d it/autofix_cli/fixed.d
# check that `dscanner @myargs.rst` reads arguments from file
section "Test @myargs.rst"
echo "-f" > "myargs.rst"
echo "github" >> "myargs.rst"
echo "lint" >> "myargs.rst"
echo "it/singleissue.d" >> "myargs.rst"
diff it/singleissue_github.txt <(../bin/dscanner "@myargs.rst")
rm "myargs.rst"

1
tests/it/autofix_cli/.gitignore vendored Normal file
View File

@ -0,0 +1 @@
test.d

View File

@ -0,0 +1,3 @@
void main()
{
}

View File

@ -0,0 +1,3 @@
auto main()
{
}

View File

@ -0,0 +1,38 @@
[
{
"name": "Mark function `const`",
"replacements": [
{
"newText": " const",
"range": [
24,
24
]
}
]
},
{
"name": "Mark function `inout`",
"replacements": [
{
"newText": " inout",
"range": [
24,
24
]
}
]
},
{
"name": "Mark function `immutable`",
"replacements": [
{
"newText": " immutable",
"range": [
24,
24
]
}
]
}
]

View File

@ -0,0 +1,12 @@
struct S
{
int myProp() @property
{
static if (a)
{
}
else if (b)
{
}
}
}

View File

@ -0,0 +1,96 @@
{
"classCount": 0,
"functionCount": 1,
"interfaceCount": 0,
"issues": [
{
"column": 6,
"endColumn": 12,
"endIndex": 22,
"endLine": 3,
"fileName": "it/autofix_ide/source_autofix.d",
"index": 16,
"key": "dscanner.confusing.function_attributes",
"line": 3,
"message": "Zero-parameter '@property' function should be marked 'const', 'inout', or 'immutable'.",
"name": "function_attribute_check",
"supplemental": [],
"type": "warn",
"autofixes": [
{
"name": "Mark function `const`",
"replacements": [
{
"newText": " const",
"range": [
24,
24
]
}
]
},
{
"name": "Mark function `inout`",
"replacements": [
{
"newText": " inout",
"range": [
24,
24
]
}
]
},
{
"name": "Mark function `immutable`",
"replacements": [
{
"newText": " immutable",
"range": [
24,
24
]
}
]
}
]
},
{
"autofixes": [
{
"name": "Insert `static`",
"replacements": [
{
"newText": "static ",
"range": [
69,
69
]
}
]
},
{
"name": "Wrap '{}' block around 'if'",
"replacements": "resolvable"
}
],
"column": 3,
"endColumn": 10,
"endIndex": 71,
"endLine": 8,
"fileName": "it/autofix_ide/source_autofix.d",
"index": 64,
"key": "dscanner.suspicious.static_if_else",
"line": 8,
"message": "Mismatched static if. Use 'else static if' here.",
"name": "static_if_else_check",
"supplemental": [],
"type": "warn"
}
],
"lineOfCodeCount": 3,
"statementCount": 4,
"structCount": 1,
"templateCount": 0,
"undocumentedPublicSymbols": 0
}

1
tests/it/singleissue.d Normal file
View File

@ -0,0 +1 @@
int NonMatchingName;

View File

@ -0,0 +1 @@
::warning file=it/singleissue.d,line=1,endLine=1,col=5,endColumn=20,title=Warning (style_check)::Variable name 'NonMatchingName' does not match style guidelines.