Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lint: collect unused files #22

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion lib/common/files.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ function getRef(seed, type){
return seed[type].toString(36) + type;
}


/**
* @class File
*/
Expand Down Expand Up @@ -296,6 +295,9 @@ var FileManager = function(baseURI, relBaseURI, console, flow){
this.warns = [];

this.readInfo = [];

// helpers
this.abspath = abspath;
};

FileManager.prototype = {
Expand Down
4 changes: 4 additions & 0 deletions lib/lint/command.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ var path = require('path');
var clap = require.main.require('clap');
var common = require('../common/command');
var isChildProcess = typeof process.send == 'function'; // child process has send method
var handleUnusedFiles = require('./reporter/parallel-process-unused-files');

function resolveCwd(value){
return path.resolve(process.env.PWD || process.cwd(), value);
Expand All @@ -20,6 +21,7 @@ module.exports = clap.create('lint', '[fileOrPreset]')
.option('--no-color', 'Suppress color output')
.option('--silent', 'No any output')

.option('--warn-unused-files <dir>', 'Warn about unused files for specified path. Avoid using with --js-cut-dev since it might cause to incorrect results')
.option('--filter <filename>', 'Show warnings only for specified file', resolveCwd)
.option('-r, --reporter <name>', 'Reporter console (default), checkstyle, junit',
function(reporter){
Expand Down Expand Up @@ -53,6 +55,8 @@ module.exports.getParallelOptions = function(){
return {
silent: true,
callback: function(res){
handleUnusedFiles(res);

var reporter = require(require('./reporter')[command.values.reporter]);
var data = require('./reporter/parallel-process-warns.js')(res);
console.log(reporter(data));
Expand Down
21 changes: 21 additions & 0 deletions lib/lint/helpers/flow-file-collector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
module.exports = function collectFiles(flow, callback){
var stack = [flow.files.queue[0]];
var collectedFiles = {};
var handled = new WeakSet();
var cursor;

while (cursor = stack.pop())
{
// mark file as handled
handled.add(cursor);
callback(cursor);
cursor.linkTo.forEach(function(link){
// prevent handling files that are already handled
if (link[0] && !handled.has(link[0])) {
stack.push(link[0]);
}
});
}

return collectedFiles;
};
44 changes: 44 additions & 0 deletions lib/lint/helpers/fs-files-collector.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
var path = require('path');
var fs = require('fs');

module.exports = function(base, extFilter){
var stack = [base];
var cursor;
var collectedFiles = {};

while (cursor = stack.pop())
{
if (!fs.existsSync(cursor))
continue;

var stat = fs.lstatSync(cursor);

if (stat.isSymbolicLink())
{
var resolvedLink = path.resolve(cursor, fs.readlinkSync(cursor));

stack.push(resolvedLink);
}
else if (stat.isDirectory())
{
var items = fs.readdirSync(cursor);

for (var i = 0; i < items.length; i++)
stack.push(path.join(cursor, items[i]));
}
else
{
if (extFilter)
{
var fileExt = path.extname(cursor);

if (fileExt.toLowerCase() === extFilter.toLowerCase())
collectedFiles[cursor] = true;
}
else
collectedFiles[cursor] = true;
}
}

return collectedFiles;
};
9 changes: 9 additions & 0 deletions lib/lint/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ var extract = require('../extract');
var command = require('./command');
var chalk = require('chalk');
var isChildProcess = typeof process.send == 'function'; // child process has send method
var unusedFiles = require('./unused/files');

if (isChildProcess)
process.on('uncaughtException', function(error){
Expand Down Expand Up @@ -91,6 +92,13 @@ function lint(config){
l10nInfo: true
}).concat([
function(flow){
if (options.warnUnusedFiles)
{
flow.usedFiles = unusedFiles.collectUsed(flow);
if (!isChildProcess)
unusedFiles.warn(flow);
}

flow.result = require('./reporter/process-warns')(flow.warns, options.filter);
}
]);
Expand Down Expand Up @@ -148,6 +156,7 @@ function lint(config){
event: 'done',
success: !flow.warns.length,
warnings: flow.warns,
usedFiles: flow.usedFiles,
result: flow.result
});
});
Expand Down
44 changes: 44 additions & 0 deletions lib/lint/reporter/parallel-process-unused-files.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
var collectFiles = require('../helpers/fs-files-collector');

module.exports = function handleUnusedFiles(tasks){
var basePaths = {};
var usedFiles = {};
var unusedFiles = {};

// merge used files from tasks
tasks.forEach(function(task){
if (task.result.usedFiles)
{
basePaths[task.result.usedFiles.collectPath] = true;
task.result.usedFiles.items.forEach(function(filename){
usedFiles[task.result.usedFiles.basePath + filename] = true;
});
}
});

if (Object.keys(basePaths).length)
{
var task = { name: 'unused files', result: { warnings: [] } };

// collect unused files
for (var basePath in basePaths)
{
var files = collectFiles(basePath);

for (var fileName in files)
if (!usedFiles.hasOwnProperty(fileName))
unusedFiles[fileName] = true;
}

// warn about unused files
for (var unusedFile in unusedFiles)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this loop doesn't needed and unusedFiles map is redundant

{
unusedFile = unusedFile.slice(process.cwd().length);
task.result.warnings.push({
file: unusedFile
});
}

tasks.push(task);
}
};
2 changes: 1 addition & 1 deletion lib/lint/reporter/parallel-process-warns.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module.exports = function(tasks){

failures.push({
loc: warn.loc,
message: warn.message + ' at ' + filename
message: warn.message ? warn.message + ' at ' + filename : filename
});
});
});
Expand Down
77 changes: 77 additions & 0 deletions lib/lint/unused/files.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
var flowFilesCollector = require('../helpers/flow-file-collector');
var collectFiles = require('../helpers/fs-files-collector');

function isTarget(basePath, collectPath, file){
return file.filename && (basePath + file.filename).indexOf(collectPath + '/') === 0;
}

exports.collectUsed = function(flow){
var options = flow.options;
var basePath = options.base;
var collectPath = flow.files.abspath(basePath, options.warnUnusedFiles);
var usedFiles = {};

flowFilesCollector(flow, function(file){
if (isTarget(basePath, collectPath, file))
usedFiles[file.filename] = true;

if (file.type == 'template')
{
if (file.decl.deps)
{
file.decl.deps.forEach(function(resource){
if (!resource.virtual && isTarget(basePath, collectPath, { filename: resource.url }))
usedFiles[resource.url] = true;
});
}
if (file.decl.l10n)
{
file.decl.l10n.forEach(function(item){
var l10nInfo = item.split('@');
var dictFilename = l10nInfo[1];

if (isTarget(basePath, collectPath, { filename: dictFilename }))
usedFiles[dictFilename] = true;
});
}
if (file.decl.styles)
{
file.decl.styles.forEach(function(style){
if (!style.resource && isTarget(basePath, collectPath, { filename: style.sourceUrl }))
usedFiles[style.sourceUrl] = true;
});
}
}
});

return {
basePath: basePath,
collectPath: collectPath,
items: Object.keys(usedFiles)
};
};

exports.warn = function(flow){
if (flow.options.warnUnusedFiles && flow.usedFiles)
{
var usedFilesInfo = flow.usedFiles;
var usedFiles = {};
var basePath = usedFilesInfo.collectPath;
var collectedFiles = collectFiles(basePath);

usedFilesInfo.items.forEach(function(filename){
usedFiles[usedFilesInfo.basePath + filename] = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not delete collectedFiles[usedFile] here? usedFiles is redundant

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cause it will affect flow.usedFiles
seems like it's not good

});

for (var usedFile in usedFiles)
delete collectedFiles[usedFile];

for (var unusedName in collectedFiles) {
unusedName = unusedName.slice(process.cwd().length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

require('path').relative(...) ?

flow.warn({
file: 'unused files',
message: unusedName
});
}
}
};