Giter Club home page Giter Club logo

liferay-frontend-source-formatter's Introduction

⚠️ This project is effectively deprecated, superseded by liferay-npm-scripts, which acts as a wrapper around Prettier, ESLint, stylelint, and some custom Liferay-specific rules.

check-source-formatting

NPM version build status Test coverage Build status

Jump to Section

Description

Runs common checks against your files to check for issues according to Liferay's formatting guidelines.

What it checks

This will scan through HTML(-like) files, JS files, and CSS files and detect many common patterns or issues that violate our source formatting rules, such as:

CSS (.css and .scss)

  • Properties being unalphabetized, such as color: #FFF; border: 1px solid;
  • Longhand hex codes that can be shorthanded, such as #FFFFFF
  • Unneeded units, such as 0px or 0em
  • Missing integers, in cases such as opacity: .4;
  • Lower or mixed case hex codes, such as #fff
  • Mixed spaces and tabs
  • Comma separated lists that are missing spaces, such as rgb(0,0,0)
  • Trailing new lines in nested rules
  • Misc. property formatting/spacing

HTML(-like) files, such as JSP, VM, FTL, etc

  • Attributes that are unalphabetized, such as <span id="test" class="foo">
  • Attribute values that are unalphabetized, such as <span class="lfr-foo hide">

JavaScript

  • Mixed spaces and tabs
  • Double quoted strings that should be single quoted
  • Stray debugging calls, such as console.log, console.info, etc
  • Functions that are improperly formatted, such as function (
  • Function arguments that are improperly formatted, such as .on('foo', function(event) {
  • Invalid bracket placement, such as ){ or } else {
  • Properties or methods that are non-alphabetically listed
  • Trailing commas in arrays and objects
  • Variables that are all defined in one statement
  • Variables being passed to Liferay.Language.get()
  • Trailing or extraneous newlines
  • Variable names starting with is (it ignores methods, which is valid, but properties and variables should not have a prefix of is)
  • Experimental Will try to parse JavaScript inside of aui:script blocks

What's new

As of v2, there are a few new features:

  • Custom configuration
  • Custom configuration on a per file basis or using globs
  • New, shorter command: csf
  • Now able to check on both ES6+ and JSX syntaxes
  • Uses stylelint for CSS checking, which should reduce false positives
  • Now requires Node 4+

API Changes

For any consumers of the API, there was previously no way to know when the CLI object had finished reading the files and reporting the errors, whereas now, cli.init() returns a promise so you can run anything you want to after everything has been processed.

Installation

<sudo> npm install -g check-source-formatting

Usage

V2 Deprecation notice:

The check_sf command is now deprecated in favor of the new csf command. They're identical (though check_sf will periodically warn you that it's deprecated), but check_sf will be removed in the next major version.

The simplest way to run it is:

csf path/to/file

However, you can also check multiple files at once:

find . -name '*.css' | xargs csf

I find it easier to use it with pull requests and check the files that were changed on the branch. In my .gitconfig I have:

sfm = "!f() { git diff --stat --name-only master.. | tr \"\\n\" \"\\0\" | xargs -0 -J{} csf {} $@; }; f"

When someone sends me a pull request, I check out the branch and run git sfm and it scans the changed files and reports any issues.

Options

There are some options that you can pass to the command:

--config If you pass this flag with a path to a configuration file, it will use that one instead of looking up one.

If you pass --no-config, it will disable any custom configuration look ups, and only use the defaults.

-q, --quiet will set it so that it only shows files that have errors. By default it will log out all files and report 'clear' if there are no errors.

-o, --open If you have an editor specified in your gitconfig (under user.editor), this will open all of the files that have errors in your editor.

-i, --inline-edit For some of the errors (mainly the ones that can be safely changed), if you pass this option, it will modify the file and convert the error to a valid value.

-l, --lint If you don't pass anything, --lint defaults to true, which switches on the linting of the JavaScript.

If you pass --no-lint it will overwrite the default and disable linting.

--junit [pathToFile] If you wish to output the results of the scan to a JUnit compatible XML file (useful for allowing integration servers to automate the check and output the results). If you don't pass the path to the file, it will default to "result.xml".

--filenames Print only the file names of the files that have errors (this option implies --quiet). This is useful if you wish to pipe the list of files to other commands.

--f, --force Formatters can choose to ignore certain files (for example, the JS formatter ignores files that end with -min.js). If you want to force the formatter to run on that file, pass this option.

Experimental or less used options

-r, --relative This will display the files passed in as relative to you current directory. This is mainly useful when you want to show the file name relative to your current working directory. I use it mainly when I'm running this on a git branch. In order for it to work from your alias, you would need to change the above alias to be:

sfm = "!f() { export GIT_PWD=\"$GIT_PREFIX\"; git diff --stat --name-only master.. | tr \"\\n\" \"\\0\" | xargs csf $@; }; f"

-v, --verbose This will log out any possible error, even ones that are probably a false positive. Currently, there's only one testable case of this, which is where you have something like this:

<span class="lfr-foo"><liferay-ui:message key="hello-world" /></span>

This will say there's an error (that the attributes are out of order), but since we can tell there's a >< between the attributes, we know that it's more than likely not a real error.

However, passing -v will show these errors (marked with a **).

--color, --no-color If you don't pass anything, --color defaults to true, which colors the output for the terminal. However, there may be times you need to pass the output of the script to other scripts, and want to have the output in a plain text format.

If you pass --no-color it will overwrite the default and give you plain text.

--lint-ids Previously, all errors that were generated by ESLint or stylelint included their rule ID in the error message. This is no longer true by default (as it adds a lot of noise), but you can turn it on by adding this option.

-m, --check-metadata If we're inside of a portal repository, and one of the files is in the /html/js/liferay/ directory, check all of the modules in that directory, and see if the requires metadata in the files matches the metadata in the modules.js file.

--show-columns If this is passed, it will show the column where the error has taken place. At this time, it only works on JavaScript, since ESLint will give us column information, but I may work something out for this in the future. This one is added mainly for Sublime Linter and other scripts that may wish use the information. It defaults to false, since it's not super useful in regular usage (at least I haven't found it to be).

If you pass -v, it will give you the lines in each file, as well as a merged version (useful for copy/pasting to update the metadata).

--fail-on-errors If this is passed, and any files report errors, this will send a non-zero exit code. This is useful if you're using it from the command line, and wish to halt the execution of any other actions. If you are using the Node API instead of the CLI, then the results array returned from the Promise will have a property of EXIT_WITH_FAILURE set to true. Examples of how you might use this (though, probably not often, at least until I have time to distinguish errors from warnings).

CLI

csf some_file_with_errors.css --fail-on-errors && do_some_new_build_task

Node API

var cliInstance = new cli.CLI(
	{
		args: ['some_file_with_errors.js'],
		flags: {
			failOnErrors: true
		}
	}
);

cliInstance.init().then(
	function(results) {
		console.log(results.EXIT_WITH_FAILURE); // logs out 'true'
	}
);

Sublime Text Integration

There are now two ways you can integrate this module with Sublime Text:

As a build system

You can setup a build system in Sublime Text to run the formatter on the file you are working in, and this is the simplest of the options. You can set up your build system using the following steps:

  • Go to the menu labeled Tools > Build System > New Build System
  • Use the following code for the new build system and save the file
{
	"shell_cmd": "csf $file"
}
  • Go to Tools > Build System and select the system you created

And that's it. When you build the file (Tools > Build) the result will be shown in the Sublime Text command line. You can also install SublimeOnSaveBuild plugin via Package Control to automatically build the file when you save it. Thanks to Carlos Lancha for the idea and docs.

As a Sublime Linter plugin

You can also use Sublime Linter to visually see in your code where the errors are: Sublime Linter

You can install it via a couple of steps:

  • Install Sublime Linter via Package Control
  • Install the SublimeLinter-contrib-check-source-formatting package via Package Control
  • In order to get it to lint, you may need to either manually lint it (in the Command Palette, type "Lint this view" and select it), or you may wish to change when it lints (in the Command Palette again, type "Choose Lint Mode", and select when you want it to lint the file).

You can read more on the project page. Thanks to Drew Brokke for writing the plugin and publishing it.

As an Atom Linter plugin

You can read more on the github page. Thanks to Michael Hadley for writing the plugin and publishing it.

Custom configuration

Starting in version 2, you can now customize the configuration of the engine in a few different ways. Here are the items you can currently customize:

  • ESLint rules
  • stylelint rules

I'm planning on expanding this into more areas, but currently those are the two sections that can be modified.

How do you define a custom configuration?

We are currently using cosmiconfig to look for configuration files, which means you can define custom configuration by adding any one of the following files somewhere in the current working directory or one of the parent directories:

  • Inside of package.json, using a custom key of csfConfig
  • csf.config.js
  • .csfrc in JSON or YAML format

What does the structure of the configuration look like?

Let's say we create a .csf.config.js file, our configuration, at the very least, should export a JSON object like so:

	module.exports = {};

Here is an example of a config with all keys defined:

	module.exports = {
		css: {
			lint: {}
		},
		js: {
			lint: {}
		},
		html: {
			css: {
				lint: {}
			},
			js: {
				lint: {}
			}
		},
		'path:**/*.something.js': {
			js: {
				lint: {}
			}
		}
	};

(please note, if using one of the JSON files, you'll need to quote the Object keys, however, there are some benefits to using a plain JS file, which I'll mention below).

And here are the descriptions of what they do:

  • css.lint - These accept any option that can be passed to stylelint, which is really just the rule configuration.
    This means anywhere the lint object is called, you can set the rules.
  • lint - These accept any option that can be passed to ESLint, you can set here. This includes any environment or globals you wish to define, rules you wish to define, etc.
    This means anywhere the lint object is called, you can set the rules.
    The html.css.lint property is only applied for style blocks inside of HTML-like files that go through the HTML formatter. This property is merged on top of anything specified in css.lint.
    The html.js.lint property is only applied for script blocks inside of HTML-like files that go through the HTML formatter. This property is merged on top of anything specified in js.lint.

You'll also notice that a key there of path:**/*.something.js. This allows you to specify a configuration to a specific file path, or a glob referencing a file path.
Any files matching that glob will apply those rules on top of the ones inside of css.lint, js.lint, html.css.lint, and html.js.lint.

Benefits to using .js over .json?

As I mentioned before, there are some benefits to using a .js file, but mainly is that it's less strict about what can go inside of the file, so you can use comments, and unquoted keys. But also, any configuration you define with a function as the value, that function will be executed, and anything you return from there will be used as the value. This means you can dynamically configure the script at runtime.

Using ESlint plugins

You can configure ESLint to leverage specific plugins for your project. The way you would do this is slightly hokey, but it's because ESLint only looks for plugins next to where eslint itself is installed. Because of this limitation, we need a path to the plugin you wish to use.

Here's how you would set it up to use, let's say the Lodash eslint plugin:

  1. In your project, run the following command:
	npm install --save-dev eslint-plugin-lodash
  1. In your configuration file, add the following:
{
	"js": {
		"lint": {
			"plugins": ["./node_modules/eslint-plugin-lodash"]
		}
	}
}
  1. Then, in the same block, you can configure any rules from the plugin by adding a rules object, and configuring it like so:
{
	"js": {
		"lint": {
			"plugins": ["./node_modules/eslint-plugin-lodash"],
			"rules": {
				"lodash/prefer-noop": 2
			}
		}
	}
}

Right now, it's not possible to use stylelint plugins, but I'll be adding support for that soon. You can still, however, configure the rules included with stylelint.

JSX and ES6/7

We include the eslint-plugin-react by default, but we only turn it on for projects that are using JSX syntax (whether it's React or MetalJS), but you need to explicitly opt into ES6 or above in your configuration. We currently handle es7 code, but we don't enable any of the future focused rules such as prefer-template, unless you set your lint.parserOptions.ecmaVersion to 7 in your config file. This is how you would do it:

	"lint": {
		"parserOptions": {
			"ecmaVersion": 7
		}
	}

Once you have that set, it will enable the react plugin rules for JSX syntax checking, and for upgrading your code to more future focused syntax.

Known issues

The following are known issues where it will say there's an error, but there's not (or where there should be an error but there's not)

  • If you have multiple tags on one line, but separated by spaces, like the following:
<span class="lfr-foo">(<liferay-ui:message key="hello-world" />)</span>

then it will still flag that as an error

  • Spaces inside of JS comments will most of the time get flagged as mixed tabs and spaces
  • Double quotes inside of JS (like inside of a regex or in a comment), will get flagged as an error.

liferay-frontend-source-formatter's People

Contributors

bryceosterhaus avatar hhuijser avatar jbalsas avatar ling-alan-huang avatar natecavanaugh avatar wincent avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

liferay-frontend-source-formatter's Issues

Throwing "Sort variables" warning when it shouldn't

Within an .ftl file, inside a <script></script> I have code that looks like this:

var svgWidth = 100;
var circleWidth = svgWidth * 2;

It throws this warning:

Line 71: Sort variables: svgWidth circleWidth

Because circleWidth uses svgWidth it should not throw this error. It throws this same error for many other variables that are technically not sorted but can't be because they consume one another.

@natecavanaugh Are there any workarounds for this or would this be considered a bug?

Force function newline for each parameters

(Suggested by BChan)

For function calls that require multiple lines, we should place each parameter in a new line:

Valid:

fn(a, b)

Valid:

fetch().then(
  () => {
  }
)

Valid:

doThings(
  a,
  () => {
  }
)

Invalid:

doThings(a, () => {
});

Invalid:

fetch().then(() => {
});

Force/disallow new line when chaining

Yesterday I had this issue while declaring a property in a metal component:

brianchandotcom/liferay-portal#70013 (comment)

I am not sure if we should add new lines for each Config method or chain everything in a single line, and formatSource gives no feedback about this.

I think prettier (#33) measures the length of the line and/or the number of method calls and adds/removes new lines depending on the situation. Do we have strict rules (like prettier does) for this situation?

This are some issues addressed by prettier for chaining:

And a sample of how it is formatted: https://prettier.io/playground/#N4Igxg9gdgLgprEAuEBhCBbADtBMB0AygCoCCxAogAQC8VwAOlFVVgE4RalJXpQBmASwDm+QbDhsoAQwA2ACgCU+AM4w244UvwA3OQFc48gOSljigDRMmLdpwBCPPkOE2qDAuPhS5Stx9V1TT9mdwI9WUMTe3MrKDc7LFQnaBdAjSgtZQio41RY-1DEgBEUgRF-TwkfBUVK3QMjY2KCqABfEAsQThhBaBVkUGk2DgB3AAVhhAGUaR0IQQATTpAAIzZpMABrOBhCLE3NZHVDLoALGAxZAHUzwXgVA7A4Qmn7wR17gE9kcBUBrriFSSGDjDbCDDSZD8OTAroAKxUAA97BttrtCNIMHAADLiODQ2FwBHIwiaWRwACK+gg8EJsjhIAObGBbF+q2kqy+smgK3YXmuSxgZ2QAA4AAxdOzA64bLC-dhwVk6AldNhwACO+kE6rB0ghUKQMIZxJAwIwgmObFOZvJVJpdKQJ1NME5gsWwuQABYuuppIJZJp0BhIb8oLgVvpgcROTNnW02kA

/cc @jbalsas

Variables imported with aui:script are not defined

When any library is being imported inside an aui:script, the formater reports the library as undefined, and gives "'something' is not defined" warning. Here you have a sample:

https://github.com/liferay/liferay-portal/blob/master/modules/apps/layout/layout-admin-web/src/main/resources/META-INF/resources/select_basic_pages.jsp#L86-L87

In those lines metal-dom is imported as dom, and being used in the next line, but csf returns "'dom' is not defined" as message error.

Add support for column number output

SublimeLinter has support to mark the column(s) where the error occurred. It would be wonderful to have column output so that SublimeLinter can provide more accurate visual feedback.

"Error loading rule 'csf-no-use-before-define': noUse is not a function"

Updated to 2.0.0-rc.4 via @next. Unable to run now due to error below

Unhandled rejection TypeError: Error while loading rule 'csf-no-use-before-define': noUse is not a function
    at module.exports (/usr/local/lib/node_modules/check-source-formatting/lib/lint_js_rules/no_use_before_define.js:49:20)
    at /usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/eslint.js:808:25
    at Array.forEach (native)
    at EventEmitter.module.exports.api.verify (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/eslint.js:780:16)
    at runLinter (/usr/local/lib/node_modules/check-source-formatting/lib/lint_js.js:37:23)
    at module.exports (/usr/local/lib/node_modules/check-source-formatting/lib/lint_js.js:58:9)
    at Formatter.JS.Formatter.create._lint (/usr/local/lib/node_modules/check-source-formatting/lib/js.js:109:20)
    at Formatter.JS.Formatter.create.format (/usr/local/lib/node_modules/check-source-formatting/lib/js.js:45:10)
    at /usr/local/lib/node_modules/check-source-formatting/lib/html.js:201:26
    at Array.map (native)
    at Formatter.HTML.Formatter.create.formatJs (/usr/local/lib/node_modules/check-source-formatting/lib/html.js:199:25)
    at Formatter.HTML.Formatter.create.parseJs (/usr/local/lib/node_modules/check-source-formatting/lib/html.js:219:25)
    at Formatter.HTML.Formatter.create.format (/usr/local/lib/node_modules/check-source-formatting/lib/html.js:127:14)
    at _.create.processFileData (/usr/local/lib/node_modules/check-source-formatting/lib/cli.js:230:29)
    at _.create.formatFile (/usr/local/lib/node_modules/check-source-formatting/lib/cli.js:133:21)
    at _.create.onRead (/usr/local/lib/node_modules/check-source-formatting/lib/cli.js:185:16)

Unclear warning: "function declaration was used before it was defined"

When calling a function declaration before it was declared you get "'function name' was used before it was defined"

For example:

console.log(hello());

function hello() {
  return 'Hi!';
}

This warning is unclear because it is valid syntax to use a function declaration below its call site. IMHO that is one of the benefits of using function declarations over function expressions. You can have your core application at the top of the file while declaring your functions at the bottom to keep things cleaner. This is especially useful when working in a non JSX environment where files like a view.jsp can easily become very large.

I could see why this warning would be thrown for a function expression as that is indeed an error.

If this is a stylistic decision then maybe something along the lines of "Function declaration needs to be defined before call site" would be more clear.

context.report error

Running csf on other FTL files in my theme worked fine but I hit an error when trying to run against portal_normal.ftl

Stacktrace:

Unhandled rejection TypeError: context.report() called with a messageId, but no messages were present in the rule metadata.
    at /usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/report-translator.js:260:23
    at Object.report (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/linter.js:945:62)
    at checkComputedProperty (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/rules/dot-notation.js:77:25)
    at Object.MemberExpression (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/rules/dot-notation.js:116:21)
    at MemberExpression (/usr/local/lib/node_modules/check-source-formatting/lib/lint_js_rules/dot_notation.js:11:32)
    at listeners.(anonymous function).forEach.listener (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/safe-emitter.js:47:58)
    at Array.forEach (native)
    at Object.emit (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/safe-emitter.js:47:38)
    at NodeEventGenerator.applySelector (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/node-event-generator.js:251:26)
    at NodeEventGenerator.applySelectors (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/node-event-generator.js:280:22)
    at NodeEventGenerator.enterNode (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/node-event-generator.js:294:14)
    at CodePathAnalyzer.enterNode (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:608:23)
    at Traverser.enter [as _enter] (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/linter.js:1006:32)
    at Traverser._traverse (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/traverser.js:132:14)
    at Traverser._traverse (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/traverser.js:147:30)
    at Traverser._traverse (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/traverser.js:147:30)

TypeError: Cannot read property 'toLowerCase' of undefined

I have noticed that I sometimes get this error. I'm not sure exactly what file or line of code is causing this. I know that really doesn't help much, but maybe it is as simple as just checking if a and b are defined first before comparing them.

Unhandled rejection TypeError: Cannot read property 'toLowerCase' of undefined
    at compareAlpha (/usr/local/lib/node_modules/check-source-formatting/lib/rule_utils.js:48:8)
    at exports.naturalCompare (/usr/local/lib/node_modules/check-source-formatting/lib/rule_utils.js:83:12)
    at /usr/local/lib/node_modules/check-source-formatting/lib/lint_js_rules/sort_vars.js:68:20
    at Array.reduce (native)
    at EventEmitter.Program:exit (/usr/local/lib/node_modules/check-source-formatting/lib/lint_js_rules/sort_vars.js:60:30)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)
    at NodeEventGenerator.leaveNode (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/node-event-generator.js:49:22)
    at CodePathAnalyzer.leaveNode (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:627:23)
    at CommentEventGenerator.leaveNode (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/comment-event-generator.js:110:23)
    at Controller.traverser.traverse.leave (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/eslint.js:908:36)
    at Controller.__execute (/usr/local/lib/node_modules/check-source-formatting/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/usr/local/lib/node_modules/check-source-formatting/node_modules/estraverse/estraverse.js:491:28)
    at Controller.Traverser.controller.traverse (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/traverser.js:36:33)
    at EventEmitter.module.exports.api.verify (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/eslint.js:902:23)
    at runLinter (/usr/local/lib/node_modules/check-source-formatting/lib/lint_js.js:42:23)

I debugged it a bit and looks like undefined is being passed from the 'Program:exit' function

var prevName = prev.id.name;
var curName = item.id.name;

looks like prev.id is defined, but the name attr is not always defined

Running the source formatter breaks some functions calls

So, my code is:

this._selectAssetEntry(selectedAssetEntry);

and the reformatted code I get is

this._selectAssetEn tryselectedAssetEntry);

which is invalid Javascript syntax. My bet is this happens because of the try in the function name.

Add VSCode extension

Having a VSCode extension allowing linting files without running it from console would be fantastic.

I offer myself to develop this plugin, but first I wanted to say it here for the case someone else has some work on it.

"Missing 'key' prop for element in iterator" with metal.js

In Metal.js we use "refs" for components when iterating through rather than a key. The linter should check for either ref or key for metal.js comonents. Metal will internally set key to ref for components.

// For component
array.map(
     (item, i) => (
           <MetalComponent ref={i} />
     )
)

// For element
array.map(
     (item, i) => (
           <div key={i} />
     )
)

Less verbose output

Currently, output for csf is very verbose. This is particularly harmful when a lot of files are scanned and few errors (if any) are found....

File: src/main/resources/META-INF/resources/js/actions/actions.es.js
    	No errors

File: src/main/resources/META-INF/resources/js/components/dialogs/SelectMappingDialog.es.js
    	No errors

File: src/main/resources/META-INF/resources/js/components/dialogs/SelectMappingTypeDialog.es.js
    	No errors

File: src/main/resources/META-INF/resources/js/components/edit_mode/DisabledAreaMask.es.js
    	No errors

File: src/main/resources/META-INF/resources/js/components/edit_mode/DisabledAreaPopover.es.js
    	No errors

File: src/main/resources/META-INF/resources/js/components/edit_mode/EditModeWrapper.es.js
    	No errors

File: src/main/resources/META-INF/resources/js/components/floating_toolbar/background_color/FloatingToolbarBackgroundColorPanel.es.js
    	No errors

It would be good to be able to skip the output if no errors are found, create some type of aggregated digest or maybe have different modes --verbose, --polite, --silent... ?

Enforce/disallow HTML void tags' slash

Since HTML5, we do not need to add an ending slash in void HTML tags. In JSX we do need to add them, as they are required by the compiler, but in Soy we should either enforce or disallow using them:

Invalid

<img src="" />
<input type="text">

Valid

<img src="">
<input type="text">

or

<img src="" />
<input type="text" />

Using more linters

I love that you’re using ESLint to handle code conformance, but I see that it’s targeting correctness and style conformity.

I recommend checking out JSCS. It’s a style linter that focuses on enforcing your style guide. It might be nice to separate error prevention and pattern conformity with styling.

There’s a lot a great tool for enforcing style conformity in CSS files. It’s called StyleLint. It has hundreds of rules that define how you want CSS to look.

There are Atom and Sublime plugins for ESLint, JSCS, and StyleLint, so you can attack this from the IDE and/or the build process.

If you’re interested, I’ve also written .eslintrc, .jscsrc, and .stylelintrc files that should be close to the patterns you once taught me, if they aren’t exact. I could put those into a PR if you are interested.

Throws an error when validating JS containing destructuring of a function's return value.

I am seeing the following error when run on this piece of code:

const {a} = myFunc();

And now the error:

Unhandled rejection TypeError: Cannot read property 'toLowerCase' of undefined
    at compareAlpha (/usr/local/lib/node_modules/check-source-formatting/lib/rule_utils.js:46:8)
    at exports.naturalCompare (/usr/local/lib/node_modules/check-source-formatting/lib/rule_utils.js:81:12)
    at /usr/local/lib/node_modules/check-source-formatting/lib/lint_js_rules/sort_vars.js:68:20
    at Array.reduce (native)
    at EventEmitter.Program:exit (/usr/local/lib/node_modules/check-source-formatting/lib/lint_js_rules/sort_vars.js:60:30)
    at emitOne (events.js:82:20)
    at EventEmitter.emit (events.js:169:7)
    at NodeEventGenerator.leaveNode (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/node-event-generator.js:49:22)
    at CodePathAnalyzer.leaveNode (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:627:23)
    at CommentEventGenerator.leaveNode (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/comment-event-generator.js:110:23)
    at Controller.traverser.traverse.leave (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/eslint.js:908:36)
    at Controller.__execute (/usr/local/lib/node_modules/check-source-formatting/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/usr/local/lib/node_modules/check-source-formatting/node_modules/estraverse/estraverse.js:491:28)
    at Controller.Traverser.controller.traverse (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/util/traverser.js:36:33)
    at EventEmitter.module.exports.api.verify (/usr/local/lib/node_modules/check-source-formatting/node_modules/eslint/lib/eslint.js:902:23)
    at runLinter (/usr/local/lib/node_modules/check-source-formatting/lib/lint_js.js:42:23)

New lines are being added between comments and logic

The npmRunFormat Gradle task adds a new line after each JSDoc comment. However, it's customary to have the comment directly above the logic. For example, do this:

/**
 * Creates a new Person.
 * @class
 */
function Person() {
}

instead of this (which is what the npm formatter is currently enforcing):

/**
 * Creates a new Person.
 * @class
 */

function Person() {
}

This is also a general rule for our Javadocs too.

The formatter should be updated to allow the comments to be directly above the logic.

Double output?

When notifying of mixed tabs and spaces, two lines will be printed:

    Line 62: Mixed spaces and tabs.
    Line 62: Mixed spaces and tabs: if (Surface.isActionURL(event.path)) {

Is this intentional?

"Args should each be on their own line" error happening when it shouldn't

Getting the two errors seen below. Bug might stem from arguments of the inner function are being required to be on their own line.

export default DragDropContext(HTML5Backend)(     //Args should each be on their own line (args on start line): DragDropContext(...)
    DropTarget(NativeTypes.FILE, specObj, collectFunc)(   //Args should each be on their own line (args on start line): DropTarget(...)
        connect(
            state => ({dirtyState: state.get('dirtyState')}),
            {
                addDirtyState,
                removeDirtyState
            }
        )(PostBase)
    )
);

Force newline between different "code blocks"

We should separate different "code blocks" (variable declarations, assignments, loops, function calls...) with new lines:

Invalid:

let a = null;
b = 3;
if (things) {
}
fn(a, b)

Valid:

let a = null;

b = 3;

if (things) {
}

fn(a, b)

Force more consistent JSDoc typing

Force using uppercase or lowercase in JSDoc primitive types, and disallow mixing box cases. This will improve documentation consistency

Invalid

/**
 * @param {object} a
 * @param {array} b
 * @param {Object} c
 * @param {Array} d
 * @param {Object[]} e
 * @param {Array<String>} f
 * ...
 */

Valid

/**
 * @param {Object} a
 * @param {Array} b
 * @param {Object} c
 * @param {Array} d
 * @param {Object[]} e
 * @param {Array<String>} f
 * ...
 */

or

/**
 * @param {object} a
 * @param {array} b
 * @param {object} c
 * @param {array} d
 * @param {object[]} e
 * @param {Array<string>} f
 * ...
 */

The source code did not change at all. Why?

iyagi@iyagi-Inspiron-7559:~/Downloads/liferay-frontend-source-formatter-2.0.10$ csf /home/iyagi/NetBeansProjects/aaaa/sum.php

iyagi@iyagi-Inspiron-7559:~/Downloads/liferay-frontend-source-formatter-2.0.10$ csf -V
2.0.10

Force new line on HTML attributes when there are more than two properties (Soy, JSX)

When there are more than two properties in a HTML tag we should force adding a new line between them, so the code is more readable:

Invalid

<form class="form form-sample" id={formId} method="POST">
  <input class="input input-minimal input-sample" placeholder="placeholder" type="text" value="sample">
</form>

Valid

<form
  class="form form-sample"
  id={formId}
  method="POST"
>
  <input
    class="input input-minimal input-sample"
    placeholder="placeholder"
    type="text"
    value="sample"
  >
</form>

Wrap JSDoc comments to 80 columns

We currently have to manually wrap JSDoc comments to follow our 80 column wrapping rule. It would be very useful (and time saving) if the tool could enforce this wrapping automatically.

Request invalid order inside aui:script taglib

For aui:script require attribute we can specify the dependency name that will be used inside the tag like:

<aui:script require="metal-dom/src/all/dom as dom">

source-format produces an error:

> Sort attribute values: metal-dom/src/all/dom as

and requires it to be sorted as:

<aui:script require="as dom metal-dom/src/all/dom">

which is actually an invalid syntax.

/cc @jbalsas

TypeError when using Freemarker variables in JS code

This freemarker code leads to a Type Error:

<script type="text/javascript"> var OSBForm${article_namespace}; </script>

Unhandled rejection TypeError: Cannot read property 'type' of null
at collectedReport.forEach (/Users/allenziegenfus/.nvm/versions/node/v6.4.0/lib/node_modules/check-source-formatting/lib/lint_js_rules/no_unused_vars.js:58:72)

Integrate with Prettier

What would it take to integrate with Prettier or achieve a prettier type functionality where code is formatted on save?

Do you think it would be more feasible to try to write this functionality from scratch ourselves?

The dev experience is absolutely amazing using prettier and increases efficiency significantly.

Return non zero on errors/warnings

Hi,

I am running gradle formatSource on a module with a package.json file containing:

"scripts": {
  "build": "cross-env NODE_ENV=production babel --source-maps -d classes/META-INF/resources src/main/resources/META-INF/resources && liferay-npm-bundler",
  "checkFormat": "npm run csf",
  "csf": "csf src/**/*.js",
  "format": "npm run csf -- -i"
}

I can see that it does run npm run csf -- -i but the build passes successfully and no warnings are printed to the console.

If I run it directly (without gradle) it does print warnings but returns 0.

Would it be possible to add a parameter so that a non zero value is returned when there are errors / warnings that must be fixed?

Thanks!

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.