Fix some code quality and bug-risk issues

[pnijhara]

• Remove length check in favour of truthiness of the object
• Fix dangerous default argument
• Remove unnecessary comprehension

---

Find the other issues found here - https://deepsource.io/gh/pnijhara/autobahn-python/issues/?category=recommended

This PR also adds .deepsource.toml configuration file to run DeepSource analysis on the repo with. Upon enabling DeepSource, the analysis will run on every PR and commit to detect 560+ types of issues in the changes — including bug risks, anti-patterns, security vulnerabilities, etc.

To enable DeepSource analysis after merging this PR, please follow these steps:

1. Signup on DeepSource with your GitHub account and grant access to this repo.
2. Activate analysis on this repo here.

You can also look at the docs for more details. Do let me know if I can be of any help!

[oberstet]

as you mention, the change only makes sense if we subscribe to DeepSource

adding DeepSource requires to grant the following permissions. it's unclear to me what this list actually means, and why that is needed. if so, I would like to trigger DeepSource from GitHub. I don't need the other directly. I will not grant anything that comes with write access to anything in this repo.

also not sure how I would connect it to crossbario organization, rather than my persional oberstet account ..

[pnijhara]

The screenshot is the GitHub oAuth2 login. By authorizing DeepSource on this screen, you would be able to sign in to DeepSource using your existing GitHub account. DeepSource only need access to your verified email address, so they can identify you.
After this step, you can connect DeepSource to the crossbario organization (you will be guided after the signup step in the UI). Once you connect, DeepSource only gets read-only access to the repositories you select — it does not ask for any kind of write access. DeepSource need the read access to automatically subscribe to push events from your repository, so the moment you make a commit or a pull-request, DeepSource can start running analysis — similar to CI tools that you use like Travis.
Detailed steps: https://deepsource.io/docs/quickstart/create-account.html#connect-github

[oberstet]

ok, got it. thanks for explanation! for completeness: a later screen during the signup process describes exactly the set of permissions (which is perfect):

I've merged the PR, and only then clicked "analyze repo" on deepsource.io, and the analysis has run successfully it seems

https://deepsource.io/gh/crossbario/autobahn-python/

awesome=)

---

btw: also thanks for the actual fixes. I've had a quick glances at some of the other reported issues .. some of these are definitely worth fixing. others, I guess we need to play with settings a bit (eg line length or such)

follow up issues:

• Review/fix issues found by deepsource static code analysis #1383
• Add deepsource project badge #1384

[oberstet]

one more Q: I would give "Autofix" a try .. but it wants more permissions than only creating new PRs (which is fine), it also asks for "direct write". just PRs isn't enough?

Direct write access (if that means direct push to master or sth): nope.

The only permission I would give is: create new PRs, and change exactly those newly created PRs. essentially, exactly what anyone has. so deepsource code fork the repo and create a PR from there. zero permissions needed on the forked repo?

[oberstet]

and one more Q: how/where do I mark issues as invalid?

eg https://deepsource.io/gh/crossbario/autobahn-python/issue/PYL-E0602/occurrences is invalid (

autobahn-python/setup.py

Line 38 in 2770817

exec(f.read()) # defines __version__

)

[meejah]

"write" permission on github is indeed push access to every repo that user owns, AFAIK.

[meejah]

Is it possible to "dismiss" warnings/errors generated by the tool?

e.g. this is neat https://deepsource.io/gh/crossbario/autobahn-python/issue/PTC-W1006/occurrences (looking for "secret" in the source, I guess?) but in a method that starts with test_ in a file that starts with test_ should probably be ignored.

So being able to say "I, a human, have looked at this warning and find it to be wrong/spurious/etc" would be good. I see it also finds SHA1 hash usage -- but also that's part of an RFC so same thing applies ("I have looked, and want to continue using sha1").

[pnijhara]

Autofix app's write permissions

For the DeepSource Autofix app, the following permissions are used:

• Content (Read and Write): Repository contents, commits, branches, downloads, releases, and merges.
• Pull requests (Read and Write): Pull requests and related comments, assignees, labels, milestones, and merges.
• Metadata (Read only): Search repositories, list collaborators, and access repository metadata.

Neither DeepSource nor DeepSource Autofix GitHub apps writes to master (or) default branch except for .deepsource.toml file. All proposed Autofix changes are pushed to a separate branch and sent for approval as a pull request.

Invalid issues

For the invalid issues, you could either report to us as a false positive (or) ignore them across the repository, only the specific occurrence, only on specific file patterns.

Ignore issue

To ignore an issue, you can do one of the following:

• Add skipcq as a comment with the issue's short-code at the end of the line, or one the line above it to silence the issue. More details here - https://deepsource.io/docs/how-to/silence-issues.html#silencing-a-specific-issue
• Ignore issue for a specific file pattern or across the repository from the dashboard.

Report false positive

When you ignore an issue, there is an option to report false positive. All false positive reports are looked at by the DeepSource's analyzer team and will be resolved in a few days.

Secret exposed issues in test files

Ref: https://deepsource.io/gh/crossbario/autobahn-python/issue/PTC-W1006/occurrences

These issues are not raised on test files. The issue is raised in this case due to a mistake in test_patterns. The correct test_patterns should be **/test/**. Otherwise autobahn/wamp/test/test_auth.py won't match */test/**. Will send a pull request with the fixes in a while.

@oberstet @meejah

[pnijhara]

Refer #1385

[oberstet]

@pnijhara thanks again for explanations and help!

• Autofix: ok, I see. we can't give those permissions
• ignoring specific issues: perfect. we'll use the in-comment markers
• invalid issue: ok, this is quite cool that you promise to look at those! I might use that if I find one which makes sense to file (the one in Fix some code quality and bug-risk issues #1379 (comment) is indeed invalid - but I can't imagine how you could detect it automatically, so won't file that one - just mark it invalid in our code)