D Dscanner Support

[nordlow]

First try at support for https://github.com/Hackerpilot/Dscanner in d-mode.

I have verified them indvidually using flycheck-select-checker.

[nordlow]

Stylecheck is nice to have though since Dscanner currently doesn't accept both arguments at the same time, which I find strange. Therefore I have defined two new checkers

• d-dscanner-syntax
• d-dscanner-style

What do you say?

[nordlow]

Problem: I can't get flycheck to use both the d-dmd and the new d-dscanner-.* checkers automatically on after the other in d-mode.

M-x flycheck-compile only displays results from d-dmd like for example

dmd -vcolumns -debug -o- -wi -I/home/per/Work/justd  /home/per/Work/justd/fs.d

eventhough flycheck-checkers contains them all locally in my D source buffer.

I thought defining :next-checkers in d-dmd would solve it but it didn't. Have I missed something?

[swsnr]

How is dscanner syntax check different from d-dmd? Do we really need both?

[swsnr]

flycheck-compile only runs the first checker in a chain. In your case, that's d-dmd.

If you want a different syntax checker, you need to select it first, via C-c ! s. Yes, that's pretty stupid. It's a relict from before multiple checkers were introduced. I opened #428 to improve it.

[swsnr]

@nordlow flycheck-compile now prompts for a syntax checker. If you rebase onto master, you can easily test your new checkers now.

[nordlow]

The three checkers are currently mostly mutally exclusive. I would like to have all of them available at once when possible :)

[swsnr]

@nordlow And how exactly is dscanner --syntaxCheck different from d-dmd? I mean, what syntax errors will it find that dmd doesn't?

If you add proper :next-checkers, Flycheck will use all three syntax checkers in a D buffer, in automatic syntax checks, and in flycheck-buffer.

It's just flycheck-compile, which can only use a single checker, mostly because I did not bother to implement chaining in it. flycheck-compile is for testing purposes mostly, i.e. matching the output against the error patterns, etc., and chaining is typically not required for that. You shouldn't use flycheck-compile as part of your regular workflow.

[nordlow]

Ok. I rebased. You're improvement works fine. But I would still prefer multiple checkers by default when :next-checker is defined. What role does :next-checker play now? Aren't multiple checkers currently activate for some modes?

[nordlow]

I don't understand why only the d-dmd checker becomes activate on my dscanner branch. Haven't I used :next-checker correctly?

[swsnr]

@nordlow Sorry, but I can't follow you. What wasn't clear about my previous comment? Again, :next-checkers is used for regular syntax checks, just not for flycheck-compile.

Specifically, flycheck-compile works totally different from normal syntax checking, because it essentially just turns the syntax checker into a shell command and passes that to M-x compile. This entirely bypasses the whole Flycheck machinery for syntax checking, including the whole chaining mechanism.

[nordlow]

I'm not talking about a flycheck-compile, I'm talking about what flycheck-list-errors displays. It only shows the output from the d-dmd checker. As I said earlier the d-mode local value of flycheck-checkers includes all D three checkers: d-dmd, d-dscanner-style, d-dscanner-syntax.

[swsnr]

@nordlow Oh sorry, I misunderstood you then. I'm sorry.

I am not sure what's wrong. The setup looks correct to me, but I did not try, for I do not use D and don't have it installed on my system. I'd need a bit more information:

• Do all three checkers work correctly in M-x flycheck-compile? Notably, is the output properly parsed, that is, are the errors highlighted in the resulting compilation buffer?
• Are all three checkers contained in flycheck-checkers, as in C-h v flycheck-checkers?

[nordlow]

The answer is yes on all your questions :)

[nordlow]

I did some more testing and found that only the first :next-checker gets run, not both.

This

:next-checkers ((no-errors . d-dscanner-style)
                  (no-errors . d-dscanner-syntax))

causes only d-dscanner-style to run and this

:next-checkers ((no-errors . d-dscanner-syntax)
                  (no-errors . d-dscanner-style))

causes only d-dscanner-syntax to run.

I believe both should run.

I also tried

:next-checkers ((no-errors . (d-dscanner-syntax d-dscanner-style)))

but that errors as

  signal(error ("(no-errors d-dscanner-syntax d-dscanner-style) must be a symbol or a cons cell"))
  error("%S must be a symbol or a cons cell" (no-errors d-dscanner-syntax d-dscanner-style))
  flycheck-validate-next-checker((no-errors d-dscanner-syntax d-dscanner-style))
  flycheck-verify-checker-properties(d-dmd)
  flycheck-set-checker-properties(d-dmd ((flycheck-documentation . "A D syntax checker using the DMD compiler.\n\nSee URL `http://dlang.org/'.") (flycheck-executable-var . flycheck-d-dmd-executable) (flycheck-command "dmd" "-debug" "-o-" "-wi" (eval (s-concat "-I" (flycheck-d-base-directory))) (option-list "-I" flycheck-dmd-include-path s-prepend) source) (flycheck-error-parser . flycheck-parse-with-patterns) (flycheck-error-patterns ("^\\(?1:.+?\\)(\\(?2:[[:digit:]]+\\)): Error: \\(?4:.+\\)$" . error) ("^\\(?1:.+?\\)(\\(?2:[[:digit:]]+\\)): \\(?:Deprecation\\|Warning\\): \\(?4:.+\\)$" . warning)) (flycheck-error-filter . flycheck-sanitize-errors) (flycheck-modes d-mode) (flycheck-predicate) (flycheck-next-checkers (no-errors d-dscanner-syntax d-dscanner-style)) (flycheck-file . "/home/per/Work/elisp/flycheck-nordlow/flycheck.el")))
  byte-code("\300\301!\302\301\303\304\305�B!#\266�\306\305\307\310\311DD\312\313\314\315\316\317\320\321\322&�\210\323\305!\210\306\324\307\310\325DD\326\315\327\313\330\331\332& \210\323\324!\210\333\301\334\335\336\337\340B\341\342\343B\344\345\346B\347\350\351 B\257\n\"\210\306\352\307\310\353DD\354\315\327\313\330\331\332&   \210\323\352!\210\333\355\356\357\360\337\340B\361\342\343B\362\345\346B\363\350\351 B\257\n\"\210\306\364\307\310\365DD\366\315\327\313\330\331\332&   \210\323\364!\210\333\367\370\371\372\337\340B\373\342\343B\374\345\346B\375\350\351 B\257\n\"\210\306\376\307\310\377DD\201@�\315\327\313\330\331\332& \210\323\376!\210\333\201A�\201B�\201C�\201D�\337\340B\201E�\342\343B\201F�\345\346B\201G�\350\351 B\257\n\"\207" [flycheck-checker-option-vars d-dmd put flycheck-option-vars -uniq flycheck-dmd-include-path custom-declare-variable funcall function #[0 "\300\207" [nil] 1 "\n\n(fn)"] "A list of include directories for dmd.\n\nThe value of this variable is a list of strings, where each\nstring is a directory to add to the include path of dmd.\nRelative paths are relative to the file being checked.\n\nThis variable is an option for the syntax checker `d-dmd'." :group flycheck-options :type (repeat (directory :tag "Include directory")) :safe flycheck-string-list-p :package-version (flycheck . "0.18") make-variable-buffer-local flycheck-d-dmd-executable #[0 "\300\207" [nil] 1 "\n\n(fn)"] "The executable of the d-dmd syntax checker.\n\nEither a string containing the name or the path of the\nexecutable, or nil to use the default executable from the syntax\nchecker declaration.\n\nThe default executable is \"dmd\"." (choice (const :tag "Default executable" nil) (string :tag "Name or path")) flycheck-executables :risky t flycheck-set-checker-properties (flycheck-documentation . "A D syntax checker using the DMD compiler.\n\nSee URL `http://dlang.org/'.") (flycheck-executable-var . flycheck-d-dmd-executable) (flycheck-command "dmd" "-debug" "-o-" "-wi" (eval (s-concat "-I" (flycheck-d-base-directory))) (option-list "-I" flycheck-dmd-include-path s-prepend) source) flycheck-error-parser flycheck-parse-with-patterns (flycheck-error-patterns ("^\\(?1:.+?\\)(\\(?2:[[:digit:]]+\\)): Error: \\(?4:.+\\)$" . error) ("^\\(?1:.+?\\)(\\(?2:[[:digit:]]+\\)): \\(?:Deprecation\\|Warning\\): \\(?4:.+\\)$" . warning)) flycheck-error-filter flycheck-sanitize-errors (flycheck-modes d-mode) flycheck-predicate nil (flycheck-next-checkers (no-errors d-dscanner-syntax d-dscanner-style)) flycheck-file flycheck-current-load-file flycheck-d-dscanner-syntax-executable #[0 "\300\207" [nil] 1 "\n\n(fn)"] "The executable of the d-dscanner-syntax syntax checker.\n\nEither a string containing the name or the path of the\nexecutable, or nil to use the default executable from the syntax\nchecker declaration.\n\nThe default executable is \"dscanner\"." d-dscanner-syntax (flycheck-documentation . "A D syntax checker using Dscanner.\n\nSee URL `https://github.com/Hackerpilot/Dscanner/'.") (flycheck-executable-var . flycheck-d-dscanner-syntax-executable) (flycheck-command "dscanner" "--syntaxCheck" source) (flycheck-error-patterns ("^\\(?1:.+?\\)(\\(?2:[[:digit:]]+\\):\\(?3:[[:digit:]]+\\))\\[error]: \\(?4:.+\\)$" . error) ("^\\(?1:.+?\\)(\\(?2:[[:digit:]]+\\):\\(?3:[[:digit:]]+\\))\\[warn]: \\(?4:.+\\)$" . warning)) ...] 13)
  load("/home/per/Work/elisp/flycheck-nordlow/flycheck.elc" nil nil t)
  load-file("/home/per/Work/elisp/flycheck-nordlow/flycheck.elc")
  query-load-buffer()
  do-on-save-elisp()
  run-mode-specific-after-save-buffer-hooks()
  run-hooks(after-save-hook)
  basic-save-buffer()
  save-buffer()
  save-buffer-and-update-tags-tables()
  funcall-interactively(save-buffer-and-update-tags-tables)
  call-interactively(save-buffer-and-update-tags-tables nil nil)
  command-execute(save-buffer-and-update-tags-tables)

[swsnr]

@nordlow Ah, now I see the misunderstanding. You need a :next-checkers keyword on d-dscanner-syntax as well. Look at the Go checkers for examples.

[nordlow]

So what effect does multiple entries in :next-checkers have then? If none then I believe the plural s in :next-checkers is misleading.

Anyhow, should I change d-dmd to use

:next-checkers ((no-errors . d-dscanner-syntax))

and d-scanner-syntax to use

:next-checkers ((no-errors . d-dscanner-style))

instead then?

[nordlow]

Now I'm done :)

Ready for merge?

[swsnr]

Shouldn't this be ((warnings-only . d-dscanner-style))? Does it make sense to apply the style checker to a buffer with syntax errors?

[swsnr]

@nordlow You didn't answer my question yet:

And how exactly is dscanner --syntaxCheck different from d-dmd? I mean, what syntax errors will it find that dmd doesn't?

[swsnr]

@nordlow And we need test cases for these new syntax checkers. I can write them myself, but I need code examples that trigger errors and warnings with these checkers.

[nordlow]

It's not easy describe the differences between DMD's and Dscanner's syntax check in a few words as I have just recently discovered the tool. Currently DScanners' syntax check gives false positives as it is not completely in sync with language standard (recent changes to DMD). Therefore I think style check should be activated eventhough syntax checking gives (false) warnings.

[swsnr]

@nordlow Try it, nonetheless. If I am to maintain these syntax checkers as part of Flycheck, you'll need to convince me that they are really necessary. And I'll want a good reason to merge a syntax checker that gives false positives because it isn't in sync with the language standard.

[nordlow]

In contrast to DMD, dscanner --styleCheck detects (copied from https://github.com/Hackerpilot/Dscanner):

• Old alias syntax (i.e "alias a b;" should be replaced with "alias b = a;").
• Implicit concatenation of string literals.
• Complex number literals (e.g. "1.23i").
• Empty declarations (i.e. random ";" characters)
• enum array literals in struct/class bodies
• Avoid Pokémon exception handling
• opCmp or opEquals, or toHash not declared "const".
• Format numbers for readability.
• delete keyword is deprecated.
• "fish operators" (floating point operators) are deprecated.
• Left side of a foreach or foreach_reverse range expression is larger than the right.
• Left side of a slice expression is larger than the right
• Variable, struct, class, union, module, package, and interface names that do not comply with Phobos style guidelines
• Struct constructors that have a single parameter that has a default argument.
• Assign expressions where the left side of the '=' operator is the same as the right
• 'if' statements where the 'else' block is the same as the 'if' block.
• Unused variables.
• Unused parameters (check is skipped if function is marked "override")
• Duplicate attributes
• Declaring opEquals without toHash

It is a great complementary tool in most regards which I'm certain most D developers would love to have integrated into flycheck.

[nordlow]

In order to have a test suite for it you need to install it right?

Do you want guidance on how to do this?

[swsnr]

@nordlow Well, that is a very convincing reason! I'll merge it.

In order to have a test suite for it you need to install it right?

Why, yes. Does that impose a problem?

Flycheck has Ansible playbooks in playbooks/ that provision Travis CI and a local testing VM based on Vagrant and Virtualbox. Both are based on Ubuntu 12.04, so if you can install it on this system, that's totally sufficient for unit testing.

[nordlow]

Not, not a problem. I was just curious how you solve it :)

I say this is ready for merge! :)

[swsnr]

@nordlow How do you build this thing? The instructions mention a build.sh, which doesn't exist. I tried make dmd, but that takes ages and is eventually killed for excessive memory use.

If we can't get Dscanner to build on Travis CI and in the local testing VM, I won't merge this PR, since I cannot reasonably maintain syntax checkers whose test cases I can't run. In this case, you'll need to maintain these syntax checkers yourself, outside of Flycheck.

[nordlow]

Are you using DMD 2.066?

[swsnr]

@nordlow Yes, of course. Don't you remember that we recently upgraded Flycheck to DMD 2.066?

[nordlow]

Could you show me the output? Update: Ahh, you didn't get any. I'll see what happens on my system. Hang on. I haven't rebuilt Dscanner in a while.

[swsnr]

@nordlow Yes, I didn't get any output. make dmd silently runs for about two minutes, before it's killed by the kernel.

[nordlow]

Killed by the kernel!? That sound like a useful feature instead of infinite swapping :) Is it a (kernel) add-on?

[nordlow]

Did you use Dscanner git master?

[swsnr]

@nordlow No, it's not. The stock Linux kernel automatically kills processes with excessive memory footprint in low memory situations.

[nordlow]

Hmm, building Dscanner Git-master with DMD Git-master and 2.066 both errors as:

make DMD=/usr/bin/dmd
/usr/bin/dmd -O -release -inline -ofdscanner  -Ilibdparse/src main.d stats.d imports.d highlighter.d ctags.d astprinter.d outliner.d symbol_finder.d libdparse/src/std/*.d libdparse/src/std/d/*.d analysis/*.d inifiled/source/*.d
astprinter.d(89): Error: no property 'asmInstruction' for type 'const(AsmInstruction)'
astprinter.d(92): Error: no property 'asmInstruction' for type 'const(AsmInstruction)'
analysis/asm_style.d(32): Error: no property 'line' for type 'const(AsmBrExp)'
analysis/asm_style.d(32): Error: no property 'column' for type 'const(AsmBrExp)'

It is strange that I don't get the same error as you do.

I guess we'll have to postpone it a while until things stabilize, I guess.

[swsnr]

@nordlow Yes. Just a straight clone from Github. I wanted to use releases, but there some to be none.

[nordlow]

I had forgotten to update the Git submodules of Dscanner.

It builds ok now both with DMD Git master and 2.066.

The compilation takes about 1 Gb of memory as DMD is currently a bit memory demanding for these kinds of projects.

How much memory have you got?

[swsnr]

@nordlow Do you build on 32 bit or 64 bit?

[nordlow]

On 64-bit. I have 8 Gigs of RAM of mine.

Do know of command to track and log min/max of the memory usage by a system call?

ps is not convenient here. I want something that prints at the of the system call kind of like time COMMAND does.

[swsnr]

time -v COMMAND should include the maximum memory usage of COMMAND. Off the top of my head I do not know how much memory the VM has, nor how much the build process used before it was killed. I'll measure.

By the way, how long did the build take?

[nordlow]

\time -v make DMD=/usr/bin/dmd

gives

##>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
/usr/bin/dmd -O -release -inline -ofdscanner  -Ilibdparse/src main.d stats.d imports.d highlighter.d ctags.d astprinter.d outliner.d symbol_finder.d libdparse/src/std/*.d libdparse/src/std/d/*.d analysis/*.d inifiled/source/*.d
##<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
    Command being timed: "remake DMD=/usr/bin/dmd"
    User time (seconds): 8.85
    System time (seconds): 0.53
    Percent of CPU this job got: 99%
    Elapsed (wall clock) time (h:mm:ss or m:ss): 0:09.40
    Average shared text size (kbytes): 0
    Average unshared data size (kbytes): 0
    Average stack size (kbytes): 0
    Average total size (kbytes): 0
    Maximum resident set size (kbytes): 1064676
    Average resident set size (kbytes): 0
    Major (requiring I/O) page faults: 2
    Minor (reclaiming a frame) page faults: 287964
    Voluntary context switches: 14
    Involuntary context switches: 2518
    Swaps: 0
    File system inputs: 0
    File system outputs: 25896
    Socket messages sent: 0
    Socket messages received: 0
    Signals delivered: 0
    Page size (bytes): 4096
    Exit status: 0

As a said, about a GB of usage.

using Dscanner commit 03b956a7bda6f51fe22b86f4f459ebfd55d92451.

[swsnr]

@nordlow Well, my VM had only ~500 MB of RAM available.

Still, the build claimed 1.3 GB at the time it was killed. That's half of what we have available on Travis CI, and more than I can reasonably provide for the local testing VM.

Also, the entire test setup, i.e. all syntax checkers, plus tools, etc, takes about 11 minutes on Travis CI currently. Building Dscanner would almost double that. That's way too long.

Are there no binary releases provided by Dscanner?

[nordlow]

No, not that I know of.

It may become integrated into https://github.com/D-Programming-Language/tools in the future.

[swsnr]

@nordlow That's unfortunate. See, even the entire local VM setup (sans downloading the initial image) takes just about 15 minutes with a good network connection, so building Dscanner for the test suite is really not an option.

I'm afraid I can't maintain Dscanner in Flycheck under these circumstances. You'll need to make a separate package for it. If you need any help, please don't hesitate to ask. Also, if you'd like to have a repo in the Flycheck org, just tell me. Take a look at the flycheck-d-unittest package for how such a package should look like.

If Dscanner provides binary builds, or becomes available as part of the D compiler, we can still move the syntax checker into Flycheck itself, but currently it's impossible. Sorry.

[nordlow]

Ok. Fine for now. Thanks anyway. I'll get back to you if binaries gets released.

[CyberShadow]

@nordlow Sorry, just wanted to ask if you have created a package which integrates DScanner with FlyCheck. I think ideally it would first run it through dmd -o- (FlyCheck's current mode) to identify errors, and if that succeeds, run it through DScanner, without requiring the user to switch between the two.

[nordlow]

No I haven't.

I'd recommend cherry picking from

https://github.com/nordlow/flycheck

Just do a diff against against upstream repo.

It contains dscanner plus python-pycompile support.