Improve Error Output

[cjnickel]

I've created two branches (feature/improve-error-output & chris/poc/context-and-reporter) but really want to get your opinion before I continue down this path. Didn't know if you already had a plan in your head how to accomplish improving error output and easier unit testing.

What I was wanting to do was mimic eslint's output for errors and warnings.

ESLint Output

Drawing from how eslint rules are written they are a JS object similar to this:

module.exports = {
    meta: {
        docs: {
            description: "some description",
            category: "some category",
            url: "documentation url"
        },
        // json object array that describes configuration options
        schema: [
        ],
        // message key value pairs for output
        messages: {
            someKey: "some message for output",
        }
    },
    create(context) {
        // report rule violations
    }
};

I like the idea of meta.docs.url with a link back to this repo with a readme explaining the particular rule but not necessary at the moment.

To start I am basically interested in the create function that provides a common interface for each rule to be called from the eslint runner. In my 2nd branch, chris/poc/context-and-reporter, I started to explore having a context object that provides a way to access the mule project's folderInfo, pomInfo, and a way to read a file within that folderInfo.
Then I created a reporter that is also passed into each "rule"'s validate function. This is used to track problems found as rules are being evaluated and then it has a function to print out errors in the format of severity category description. Initially I was thinking to include a category for the error and ideally I'd like to look at adding which file the error was found.

I think these two things will allow for the rules/validation functions to be passed their mock data to more easily unit test them without having to mock the fs package or always have a file to be read.

[cjnickel]

@TrueWill - Let me know your thoughts and if possible lets hop on a call to discuss.

[TrueWill]

@cjnickel Looking initially at feature/improved-error-output, my only quibbles are:

• printProblemsSummary says those are errors; I see them as warnings. Errors are handled by fatal().
• At least with my PowerShell settings, red is hard to read:
  
  Maybe go with yellow for warnings and bright red for errors?

It's nice that it handles redirecting to a file; it doesn't give a bunch of color codes when you do that.

I did notice that the colors don't show up in Git Bash; this doesn't matter to me, but I've seen one person use it. It works fine in cmd. I didn't test on Mac/Linux.

Honestly I didn't think much about testing; this was a quick-and-dirty side project. I like the separation of the I/O from the assertions.

Want to open a PR on that?

[TrueWill]

@cjnickel On chris/poc/context-and-reporter, just abstracting readFile isn't going to be enough - see pomParser.

I'm iffy about the name "context" due to it being an overloaded term. The various assertions take in a context string that represents what file/section the check is in.

gitignore.validate(muleContext, reporter) does read well, though, and it looks like you changed context to category (more or less). The reporter module definitely seems cohesive.

This and similar paths are definitely worth exploring. I'd like it to be more modular and easier to test. I do wonder if we'll get time to write tests against it; as it is I have to work on it outside of the office or in rare slices of downtime.

[TrueWill]

(My main thought for testing was to create a sample project that had every possible warning, then run the tool on that and capture the output to use for regression testing. Basically poor man's snapshot testing.)

[cjnickel]

Console Colors

For reference this is what it looks like for me on iTerm on a Mac.

I hadn't thought about powershell's default blue theme. I'll focus on the core functionality of what information we want to capture for reporting warnings and errors without worrying about color so much to start. After that direction is figured out a bit better we can look at the actual color output.

Problems Summary / Summary

printProblemsSummary:

• I was thinking with the way reporter is setup that we can pass in a severity of either warning or error to start. I think I can hash out the structure of reporter and then we can make sure to get the severity correct.

Context

Agree with your concern about context. Within eslint's codebase context represents the AST of the file being parsed, I believe. I was trying to think of a better name/concept to represent the "mule project context" that validation is running against.

PomParser

I think this is actually handled by the context because the pom property here is the output from the pomParser. That gets passed in here in mulint.js

"as it is I have to work on it outside of the office or in rare slices of downtime."
Yup, that's why I was wanting to help you out since I saw you were doing most of the work on it over weekends. The tool is invaluable in alleviating pain points so definitely don't want you to be trying to maintain all of it on your personal time.