WIP: Add powershell/PSScriptAnalyzerBear #1967
Conversation
userzimmermann
force-pushed
the
zimmermann/powerbear
branch
from
5d148c4 to
de6594d
Compare
|
powerBears |
userzimmermann
force-pushed
the
zimmermann/powerbear
branch
2 times, most recently
from
264f35a to
860eee9
Compare
|
Hi @userzimmermann, thanks for letting me know about coala, I've been a long-time user of SonarQube (but it doesn't have plugin for Powershell) 😃 As for other PS analysis tools I had been using ScriptCop, but I believe it's not maintained any more, so I would stick with PSScriptAnalyzer. |
userzimmermann
force-pushed
the
zimmermann/powerbear
branch
from
860eee9 to
7faf29f
Compare
| ', '.join('{}={}'.format(name, name) | ||
| for name in rules)), | ||
| {'func': self.func}, | ||
| doc=self.func.__doc__, __module__ = __name__) |
There was a problem hiding this comment.
E251 unexpected spaces around keyword / parameter equals'
PycodestyleBear (E251), severity NORMAL, section autopep8.
| ', '.join('{}={}'.format(name, name) | ||
| for name in rules)), | ||
| {'func': self.func}, | ||
| doc=self.func.__doc__, __module__ = __name__) |
There was a problem hiding this comment.
E251 unexpected spaces around keyword / parameter equals'
PycodestyleBear (E251), severity NORMAL, section autopep8.
| ', '.join('{}={}'.format(name, name) | ||
| for name in rules)), | ||
| {'func': self.func}, | ||
| doc=self.func.__doc__, __module__ = __name__) |
There was a problem hiding this comment.
The code does not comply to PEP8.
PEP8Bear, severity NORMAL, section autopep8.
The issue can be fixed by applying the following patch:
--- a/bears/powershell/PSScriptAnalyzerBear.py
+++ b/bears/powershell/PSScriptAnalyzerBear.py
@@ -76,7 +76,7 @@
', '.join('{}={}'.format(name, name)
for name in rules)),
{'func': self.func},
- doc=self.func.__doc__, __module__ = __name__)
+ doc=self.func.__doc__, __module__=__name__)
# ... so they need to be implicitly added afterwards
caller.__annotations__ = {name: bool for name in rules}
caller.__defaults__ = (True, ) * len(rules)
|
@Makman2 I added this @linter(executable='powershell', settings={name: (bool, True) for name in rules()})
class PSScriptAnalyzerBear:
...
# @linter can create the wrapper here or we extend the settings discovery routines
@staticmethod
def create_arguments(filename, file, config_file, **rules):
... |
|
@mgr32 Thanks for your response :) Already tried coala in the meantime? ;) The nice advantages over SonarQube are the simple extensibility for new languages/tools, as you can see it here for PSScriptAnalyzer, and the CI integration with GitHub/Lab via @gitmate-bot which makes those nice PR comments like above in #1967 (review) and blocks the PR from getting merged until cleaned up... If you have further ideas or remarks regarding this PR or coala in general, please let us know :) Also @mczerniawski @DomBros @bielawb ... If anything comes to your mind regarding this ... Can't wait to read it ;) |
|
|
|
@Makman2 I hope I got you right ;) There just doesn't seem to be another way to define Bear settings than adding them as parameters to |
|
Actually you can override We could make those public and allow to modify those, so you can define parameters like you want. But I want to be sure that this is a good thing to do ;) However, I'm not sure that what you try to achieve for this bear specifically is a good approach. If you have different versions of PSScriptAnalyzer (not sure how powershell includes this commandlet), the bear signature changes. This is imo not desired, and we should have a fixed and static list of arguments coala supports. Also we are able to adapt style/rule names to our naming scheme (instead of taking them directly from powershell). |
|
@Makman2 Thanks! The
This is exactly what I was referring to with "I know this purely dynamical approach is not a good solution for a release installation." And with the solution approach "And then creating some static bundled setting definitions on building bears releases from it..." we could always roll out the same pre-defined settings based on the linter version in our build system
Why reinventing the wheel? And we don't have a real naming scheme for bear settings. Every linter bear developer creates them on their own, mostly manually resembling the tool's native options. Of course! Why reinventing the wheel? ;) The tool users are used to those options. And this is usually done once. Haven't seen some pull request like All that hopefully changes with aspects... Then we really have to think about the integration of linter options and need to keep the aspects<-->tool-options relations up to date |
Yes we actually have ongoing efforts to unify setting names to include for example prefixes like Another thing are the type annotations: There are settings which are boolean, some which are string, and some are integers. If it's possible to extract this information out of the documentation texts, cool. If not we can only handcraft those ;) It's technically not necessary to annotate the types as we directly pass the settings to the linter, however it's valuable documentation for the user. It's not that I want to turn down the idea, I'm just really not sure if it's the right direction for us to go. |
|
IMO this is not helpful. We need precise versions of upstream tools , managed with package managers. |
|
@jayvdb Exactly my ideas... I don't see any contradiction :) I only want to additionally generate the settings from that same precise version of
Of course! Automation where possible. Handcraft where needed. That's obvious ;) |
|
@userzimmermann , how are you going to automatically map those to aspects? And we dont want You'll end up with a custom parser that is quite complex, and most importantly that parser doesnt belong in the bear. Put it in a |
|
@jayvdb I didn't write anything about automatic mapping to aspects... only settings But it seems like manual adjustments are needed... So it looks like we could need some interactive tool-options<-->settings mapping assistant... For the development process... Something the |
While attending @PPOSHGROUP's https://www.meetup.com/Polish-PowerShell-Group-PPoSh/events/241810275/ I realized that we haven't grown any powerBear for analyzing PowerShell code yet 😨 How do we ever want to get really accepted by the Windows community w/o the
PSScriptAnalyzerBear? Here it is! And needs some mapping to all the https://github.com/PowerShell/PSScriptAnalyzer rules :D And tests of course! So there is still some work left to do...@mgr32 Hi :) I was talking about coala and PS integration with @mczerniawski and he said you might be interested in such stuff ;) Are there maybe even more PS code analysis tools worth integrating?
cc @sils