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

Fix NPE with commented import in kotlin scripts #999

Merged
merged 6 commits into from
Jul 27, 2021

Conversation

petertrr
Copy link
Member

@petertrr petertrr commented Jul 26, 2021

What's done:

  • Changed logic
  • Added tests
  • Changed runner for build_and_test workflow

This pull request closes #994

### What's done:
* Changed logic
* Added tests
@codecov
Copy link

codecov bot commented Jul 26, 2021

Codecov Report

Merging #999 (70e505e) into master (e51714b) will decrease coverage by 0.01%.
The diff coverage is 33.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #999      +/-   ##
============================================
- Coverage     83.80%   83.79%   -0.02%     
- Complexity     2422     2423       +1     
============================================
  Files           102      102              
  Lines          6108     6108              
  Branches       1809     1810       +1     
============================================
- Hits           5119     5118       -1     
  Misses          269      269              
- Partials        720      721       +1     
Flag Coverage Δ
unittests 83.79% <33.33%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
.../cqfn/diktat/ruleset/utils/indentation/Checkers.kt 74.77% <ø> (ø)
.../ruleset/rules/chapter2/kdoc/CommentsFormatting.kt 71.87% <33.33%> (-0.63%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e51714b...70e505e. Read the comment docs.

@petertrr petertrr requested review from kentr0w and kgevorkyan July 26, 2021 11:30
// In case when comment is inside of a function or class
if (node.treePrev.isWhiteSpace()) {
node.treePrev.treePrev.elementType == LBRACE
// in some cases (e.g. kts files) BLOCK doesn't have a leading brace
node.treePrev?.treePrev?.elementType == LBRACE
Copy link
Member

Choose a reason for hiding this comment

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

treePrev, treeParent are nullable types, however, they don't marked as such in source code, so IDEA don't advise to make such calls 'save' automatically, and so we use them is some dangerous way
May be we need to change such calls in whole project to appropriate !! or ?. in aim to avoid some problems in future?

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem here is that we won't get any compiler checks, unless source of ASTNode is annotated with proper annotations. For sure, adding safe access to all such unvokations may save us from some NPEs, but I don't see a good way to enforce this style.

@petertrr petertrr merged commit 39e6c8f into master Jul 27, 2021
@petertrr petertrr deleted the bugfix/gradle-script branch July 27, 2021 07:33
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.

Internal error on gradle script
2 participants