liferay/eslint-config-liferay

require CallExpressions inside MemberExpressions trick group-imports rule

wincent opened this issue · 5 comments

Just noticed this while looking at https://github.com/liferay/liferay-js-themes-toolkit/pull/393/files#diff-607f89928a61f2489713663b891f76aa

diff --git a/packages/liferay-theme-tasks/plugin/test/tasks/version.js b/packages/liferay-theme-tasks/plugin/test/tasks/version.js
index 1ebd412..1634a94 100644
--- a/packages/liferay-theme-tasks/plugin/test/tasks/version.js
+++ b/packages/liferay-theme-tasks/plugin/test/tasks/version.js
@@ -10,6 +10,7 @@ var chai = require('chai');
 var del = require('del');
 var fs = require('fs-extra');
 var Gulp = require('gulp').Gulp;
+
 var os = require('os');
 var path = require('path');

@@ -30,11 +31,11 @@ var initCwd = process.cwd();
 var registerTasks;
 var runSequence;

Looks like the Gulp require is tricking the group-imports rule into starting a new group.

Similar problem here: https://github.com/liferay/liferay-js-themes-toolkit/pull/393/files#diff-5688ba5971f6c2d3bdf9cf4265442a01

diff --git a/packages/liferay-theme-tasks/tasks/build.js b/packages/liferay-theme-tasks/tasks/build.js
index 5e06621..8fa8470 100644
--- a/packages/liferay-theme-tasks/tasks/build.js
+++ b/packages/liferay-theme-tasks/tasks/build.js
@@ -8,12 +8,13 @@

 const del = require('del');
 const fs = require('fs-extra');
-const _ = require('lodash');
-const path = require('path');
-const plugins = require('gulp-load-plugins')();
 const replace = require('gulp-replace-task');
-const through = require('through2');
+const _ = require('lodash');
+const plugins = require('gulp-load-plugins')();
+
+const path = require('path');
 const PluginError = require('plugin-error');
+const through = require('through2');

 const getBaseThemeDependencies = require('../lib/getBaseThemeDependencies');
 const lfrThemeConfig = require('../lib/liferay_theme_config');

This time the CallExpression is inside another CallExpression.

I think the fix here will be two (or maybe three parts):

  1. Teach group-imports to not add those blank lines.
  2. (Definitely) Add a new lint that tells people to prefer destructuring over x = require('foo').x.
  3. (Maybe) Add a new lint that tells people not to immediately call require results (eg. x = require('x')()).

Crude fix for "1." is up at #97

Will look at "2." (and maybe "3.") later on.

New "destructure-requires" rule to address "2." is up at: #98

New "no-require-and-call" rule to address "3." is up at: #99