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

"Source files should not have any duplicated blocks": what about property values of object arrays? #717

Closed
sebsnake opened this issue Aug 18, 2017 · 4 comments

Comments

@sebsnake
Copy link

Let's assume I have something like the following JS code somewhere in my applications source files:

const listOfObjects = [{
  prop1: 'value11',
  prop2: true,
  prop3: this.getValueForProp('value1', 3),
  prop4: 'value14',
  prop5: true
},{
  prop1: 'value21',
  prop2: false,
  prop3: false,
  prop4: 'value24',
  prop5: true
},{
  prop1: 'value31',
  prop2: true,
  prop3: this.isSomethingSetSomewhereElse() ? 'random value' : this.getValueForProp('value3', 3),
  prop4: 'value34',
  prop5: false
}];

Sonar won't stop telling me, that (in this example) the lines starting with 'prop' are duplicated code, because it seems to ignore the values. This is just a simple example, I fairly often use these object lists, e.g. showing/hiding menu items based on given rights and so on.

So, if this IS an issue, could you please adjust it to look for values as well?
But if this is NOT an issue, please tell me, what sonars official solution for this type of problem is.

@vilchik-elena
Copy link
Contributor

@sebsnake I can't reproduce the duplication.
Concerning the string literals' values, we are ignoring them on purpose: if that's the only thing which differs, probably, it's better to refactor the code.
On other side, identifier and properties' names are not ignored, so I don't see how you can get duplication detected on your code example.
Moreover there should be at least 100 tokens and 10 lines to say that code is duplicated (see doc https://docs.sonarqube.org/display/SONAR/Metric+Definitions)

@sebsnake
Copy link
Author

Hi, thanks for your answer.
I must confess, the example was made up and somehow badly chosen, excuse me for using it. I searched again for one of those sonar tickets I was refering to, and after altering the code to obfuscate anything sensitive and let sonar do his tasks on it, this code block here does produce the problem (with sonar version 6.4, revision 25310 + sonar js 3.1, build 5111):

createTaskListModified() {
  return [{
    value: 'DemoApp-Task1',
    text: translate('translationPrefix.menuItems.task1'),
    route: 'DemoApp_Task1',
    visible: User.hasRole('ROLE_ACCESS_TASK1')
  }, {
    value: 'DemoApp-Task2',
    text: translate('translationPrefix.menuItems.task2'),
    route: 'DemoApp_Task2',
    visible: User.hasRole('ROLE_ACCESS_TASK2')
  }, {
    value: 'DemoApp-Task3',
    text: translate('translationPrefix.menuItems.task3'),
    route: 'DemoApp_Task3',
    visible: User.hasRole('ROLE_ACCESS_TASK3')
  }, {
    value: 'DemoApp-Task4',
    text: translate('translationPrefix.menuItems.task4'),
    route: 'DemoApp_Task4',
    visible: User.hasRole('ROLE_ACCESS_TASK4')
  }, {
    value: 'DemoApp-Task5',
    text: translate('translationPrefix.menuItems.task5'),
    route: 'DemoApp_Task5',
    visible: User.hasRole('ROLE_ACCESS_TASK5')
  }, {
    value: 'DemoApp-Task6',
    text: translate('translationPrefix.menuItems.task6'),
    route: 'DemoApp_Task6',
    visible: User.hasRole('ROLE_ACCESS_TASK6') && this.taskIsEnabled(6)
 }, {
    value: 'DemoApp-Task7',
    text: translate('translationPrefix.menuItems.task7'),
    route: 'DemoApp_Task7',
    visible: User.hasRole('ROLE_ACCESS_TASK7')
  }, {
    value: 'DemoApp-Task8',
    text: translate('translationPrefix.menuItems.task8'),
    route: 'DemoApp_Task8',
    visible: User.hasRole('ROLE_ACCESS_TASK8')
  }];
}

Sonar tells me, that the lines 3 to 7 are duplicated by the lines 8 to 30.
Yes, it looks like an pattern you could easily write a factory function for, but I just would like to know, if this really is a ticket or a false positive. :)

@vilchik-elena
Copy link
Contributor

vilchik-elena commented Aug 22, 2017

No, it's an expected behaviour. Indeed, sometimes you prefer to avoid factory functions and want to keep such object literals as they are. In this case you could ignore specific files from duplication detection. See https://docs.sonarqube.org/display/SONAR/Narrowing+the+Focus -> "Ignore Duplications". Or you just mark issue as "won't fix" (but then duplication metric will be still the same).

@sebsnake
Copy link
Author

Ok, thats good to know.
Thank you for your time and effort. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants