Add code analysis script

[th-we]

Checks libmei for valid fields and the MEI schema for valid attributes. Reports if any non-existent ones are used. Should help with upgrading to an updated MEI schema.

Can somebody try to run npm run analyze to see if this works for them?

[rettinghaus]

Seems to work, but it only catches attributes, that are not available in MEI at all. It won't get attributes, which are not allowed on different elements, e.g. you may add @form to <note>, and it won't complain. Is there a reason, you have to run install before this?

[rettinghaus]

… and please add the changed package-lock.json to the commit or put it on .gitignore.

[th-we]

Interesting, I didn't have to do another npm install, so my package-lock.json didn't change. Those package-lock.json files are still a mystery to me. I'll have a look at this.

Of course the script doesn't catch wrong uses of attributes (that's why I wrote "it can not catch all invalid uses of element/attribute combinations" in the leading comment of the script) but I hope it can support the process of updating to a newer MEI version. At least I caught this one attribute in the process.

What would also be a good idea is to validate all output MEI files against the current schema. But not in this pull request...

[th-we]

Updated both package.json and package-lock.json. @annplaksin Could you test?

[rettinghaus]

npm complains to have

found 5 low severity vulnerabilities

Shouldn't we move to mocha 7.2.0 in package.json?

[rettinghaus]

Perhaps analyze is a bit too much as name. How about check-attributes?

[annplaksin]

Just to mention, I somehow am still not able to run npm analyze: My powershell output looks like my problem is, that I am not able to run node tools\analyze.js since it tries to resolve the command properly... It still doesn't when I try node tools\\analyze.js and node tools\analyze.js ... meaning I try, nothing happens and after a short while I have my command line back... By the way, when I cd into tools I get the error, that i doesn't find lib\libmei.plg When I copy analyze.js to the repo root, nothing happens again.

When I npm install, I get the same vulnerabilities, but nothing happens afterwards when I try to run the analysis again.

@rettinghaus Are you running this on a Linux or a Windows? And is the analysis running on your machine?

[ahankinson]

@annplaksin did you try npm run analyze?

[annplaksin]

Yep, I did that.

[rettinghaus]

@annplaksin On Linux it runs just fine.

[annplaksin]

That's what I expected... but on Windows something seems going really strange with analyze.js since I am definetively not able to run this script from the root directory via PowerShell. It should work with node folder\file.js as it does in other cases... but nothing happens in this case.

[ahankinson]

Does console.log() work in PowerShell?

https://stackoverflow.com/questions/9846326/node-console-log-behavior-and-windows-stdout

[annplaksin]

Yes it works without problem with the parentheses... as well, npm test or gulp are working fine as well.

[rettinghaus]

@th-we the package-lock.json was introduced by @ahankinson with commit 89c9fe45d41921ef47f293eaab84e995a709088f It should be deleted and added to .gitignore https://docs.npmjs.com/configuring-npm/package-lock-json.html

[ahankinson]

General best practice is to commit version lock files so that different developers can be guaranteed to be running the same versions of dependencies. The link you provide explicitly states: "This file is intended to be committed into source repositories, and serves various purposes"

[rettinghaus]

@ahankinson But it "is automatically generated". And a quick GitHub search reveals 375,656 results in ignore lists. If there aren't dependencies that have to be kept, I think it's safe to remove it …

[ahankinson]

It is automatically generated on one machine, but then used by others to pin their versions to the exact dependencies of the original author. This includes sub-dependencies of packages.

I'm not one to generally defend JavaScript dependency management (it's pretty horrid!) but I think in most cases it is best to keep it in the project.

Is it causing you specific problems?

[ahankinson]

For example, I know I've had real problems in the past with this project when 'gulp' was updated.

[rettinghaus]

No real problems. It's just annoying that it signifies me constantly about vulnerabilities and keeps getting changed …

[ahankinson]

If you fix the packages to non-vulnerable ones then everyone else gets to benefit from the updates too... If it's changed, it should be re-committed.

[ahankinson]

(Again, JS dependency management is a pretty awful experience, but the package locking process isn't the problem -- it's the sheer number of tiny dependencies that each have their own agendas and then the knock-in effects of keeping a million packages up-to-date. Deleting package-lock only hides the problem, a bit like closing the window shutters facing the burning house next door because the light is keeping you awake. :)

[rettinghaus]

If @th-we resolves the conflicts, I'm happy to merge.

[th-we]

@rettinghaus Sorry for another force-push – it was the easiest way to resolve the conflicts.

[rettinghaus]

Did you ever tried a rebase? ;-)

[ahankinson]

"Yes, sure I could have used my key, but kicking the door down with my foot was the easiest way to come inside." :-D

[th-we]

@rettinghaus I just did an interactive rebase and removed the conflicting lines from the commit.