Fix to some rustfmt::skip
issues
#4803
Open
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Suggested fix for issue #4706, including enhancements to the handling of
#[rustfmt::skip]
.#4706 issue was caused because
format_impl()
did not copy the skipped range from the localcontext
into thevisitor.context
. The same issue was corrected also informat_trait()
andrewrite_macro_with_items()
.While testing I found that there are cases where
self.line_number
inpush_skipped_with_span()
is incorrect (usually 0 or 1). That caused an incorrect skipped range to be created. Therefore, I had to make changes to this function. The main change is that the beginning of the skipped range mainly depends on the beginning of the attributes block, instead ofself.line_number
.Another major change to
push_skipped_with_span()
is related to the following comment:rustfmt/src/formatting/visitor.rs
Line 856 in 94035f9
This comment is not correct in general, as in several cases only the first attribute in an attributes block is not skipped (this was also partly discussed in PR #4707). Therefore, my change to
push_skipped_with_span()
assumes that only the first attribute line should not be skipped. If this is the correct approach, then the above comment should be removed. Otherwise,push_skipped_with_span()
should be changed so the beginning of the skipped range will depend on the end of the attributes block, instead of its beginning.I am aware that the above changes are more general than the specific #4706 issue, but there doesn't seem to be a good way to fix only this issue, because of the required change for
push_skipped_with_span()
to handle wrongself.line_number
that impacts other skip cases.Because of the general changes, the test cases file include tests for skipping fn/impl/trait/macro. In addition, a test was added in
report.rs
to make sure that the generatedskip_ranges
is correct.