danielstjules / jsinspect

Detect copy-pasted and structurally similar code
MIT License
3.57k stars 128 forks source link

Matches a little to much #5

Closed trichins closed 10 years ago

trichins commented 10 years ago

I ran jsinspect with the identifier option (I'm just looking for copy and paste) and it seems to have a broad match. Take these two files (a reduced case from two larger files).

test1.js (function () { test1 = new Class({

    build: function () {
        this.options.header = this.options.title;
        this.options.subtitle = this.options.preSubtitle;
        this.options.main_class =  this.options.mainClass;
        this.options.wrapper_class = this.options.wrapperClass;
        this.options.loading_class = this.options.loadingClass;
        this.options.id = this.options.ID;
        this.options.close_class = this.options.closeClass;

        this.options.download = this.options.download;
        this.options.downloadclass = this.options.downloadClass;

        this.options.caputured_text = this.options.capturedText;
        this.options.captured_date = new Date().format(this.options.titleDateFormat);
        this.options.view_options_class = this.options.viewOptionsClass;
        this.options.view_options = this.options.viewOptions;
        this.options.show_status_label = this.options.showStatusLabel;
        this.options.show = this.options.show;
        this.options.hide = this.options.hide;

        this.parent();

    }
});

}());

test2.js (function () { A = new Class({

    initialize: function () {
        o = o || {};

        this.options = Global.options || {};
        this.dialog = null;
        this.request = null;
        this.pollIntervalMax = 10000;
        this.pollIntervalIncrement = 1000;
        this.pollInterval = 1000;
        this.pollToken = null;
        this.pollIndex = 1;
        this.elementId = 'reputations';
        this.element = null;

        this.messages = {
            requestNoSuccess: 'request_no_success',
        };

        this.responseElements = {};
        this.responseElements2 = {};
        this.item = null;
        this.requestsPending = [];
        this.ResponseCount = 0;
    },
});

}());

If you run jsinspect -i -t 34 test1.js test2.js, it will find a match. But with the identifier flag turned on, it don't think it should have matched them.

I believe the problem is that you don't have an identifierHandler for an ExpressionStatement so it is just comparing the fact that there are 16 ExpressionStatements in both files. If I add an identifierHandler for an ExpressionStatement (by checking the node type of the left node and using its identifierHandler), it stops matching the two.

danielstjules commented 10 years ago

Thanks for pointing that out! I'll fix the behaviour as soon as I get home. Did you have a patch/diff/pr for nodeutils with those changes? If not, no worries. :)

danielstjules commented 10 years ago

@trichins Fixed in https://github.com/danielstjules/jsinspect/commit/b6238b9554e5944ea1448d05962cb2278ee4dd66 You can expect a new release soon. :)