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

SONARJS-705 Migrate plugin on SQ 5.6 LTS #245

Merged
merged 18 commits into from
Jun 3, 2016
Merged

SONARJS-705 Migrate plugin on SQ 5.6 LTS #245

merged 18 commits into from
Jun 3, 2016

Conversation

vilchik-elena
Copy link
Contributor

No description provided.

@@ -138,10 +138,19 @@ private void saveComplexityMetrics(TreeVisitorContext context) {
saveMetricOnFile(CoreMetrics.COMPLEXITY_IN_CLASSES, classComplexity);
saveMetricOnFile(CoreMetrics.COMPLEXITY_IN_FUNCTIONS, functionComplexity);

sensorContext.saveMeasure(inputFile, functionComplexityDistribution.build(true).setPersistenceMode(PersistenceMode.MEMORY));
sensorContext.<String>newMeasure()

Choose a reason for hiding this comment

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

MAJOR Make this line start at column 5. rule

@vilchik-elena vilchik-elena force-pushed the 5_6_migration branch 7 times, most recently from 721551a to d8b38b3 Compare May 31, 2016 09:40
DefaultInputFile inputFile = new DefaultInputFile("src/test/resources/metrics/lines.js")
.setAbsolutePath(file.getAbsolutePath())
DefaultInputFile inputFile = new DefaultInputFile("", "src/test/resources/metrics/lines.js")
// .setAbsolutePath(file.getAbsolutePath())
Copy link
Contributor

Choose a reason for hiding this comment

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

@vilchik-elena can we remove that comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pynicolas i think it's fixed in next commits

Copy link
Contributor

Choose a reason for hiding this comment

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

@vilchik-elena ok, sorry for the noise.

@pynicolas
Copy link
Contributor

@vilchik-elena JavaScriptProfile still uses RuleFinder which is deprecated. Is there a way to change that or do we have to live with it?

@vilchik-elena
Copy link
Contributor Author

vilchik-elena commented May 31, 2016

@pynicolas we should drop it, but i have no idea how. Let's discuss that tomorrow

@pynicolas
Copy link
Contributor

@vilchik-elena CustomJavaScriptRulesDefinition refers to BatchExtension which is deprecated: can we change that?

@vilchik-elena
Copy link
Contributor Author

@pynicolas fixed in acf6af4

@pynicolas
Copy link
Contributor

@vilchik-elena I think we can remove JavaScriptCommonRulesDecorator and JavaScriptCommonRulesEngine, see SONAR-6703.

int startOffset = token.startIndex();
int endOffset = token.toIndex();
return builder.newSymbol(startOffset, endOffset);
private static void addReference(NewSymbol symbol, InternalSyntaxToken referenceToken) {
Copy link
Contributor

Choose a reason for hiding this comment

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

@vilchik-elena minor: can't we use SyntaxToken instead of InternalSyntaxToken?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pynicolas we can now!:) btw we can drop offset properties from InternalSyntaxToken. WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vilchik-elena We can try, but I'm not sure it's easy.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vilchik-elena
Copy link
Contributor Author

@pynicolas common rules are dropped in 65a9cc7


context.fileSystem().add(inputFile);

return inputFile.initMetadata(new FileMetadata().readMetadata(inputFile.file(), Charset.defaultCharset()));
Copy link
Contributor

Choose a reason for hiding this comment

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

@vilchik-elena Charset.defaultCharset() should be replaced with a given charset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pynicolas fixed in 600ccbb

@Override
public boolean shouldExecuteOnProject(Project project) {
return fileSystem.hasFiles(mainFilePredicate);
private FilePredicate mainFilePredicate(FileSystem fileSystem) {

Choose a reason for hiding this comment

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

MAJOR Make "mainFilePredicate" a "static" method. rule

@SonarTech
Copy link

SonarQube analysis reported 5 issues

  • MAJOR 3 major
  • MINOR 2 minor

Watch the comments in this conversation to review them.

2 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MINOR LocalStorageCheck.java#L43: Remove this use of "SubCharacteristics"; it is deprecated. rule
  2. MINOR MetricsVisitor.java#L132: Remove this use of "ACCESSORS"; it is deprecated. rule

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

Successfully merging this pull request may close these issues.

None yet

4 participants