Refactor BannedElement#1506
Conversation
| @Test(expected = NullPointerException.class) | ||
| public void testBannedElementConstructor_nullLocation() { | ||
| new BannedElement("test", null, 0); | ||
| new BannedElement("test", "", 1, null, 0); |
There was a problem hiding this comment.
I would expect that "" would be flagged as an illegal marker
There was a problem hiding this comment.
Marker IDs are only used when searching the resource for markers for quick fixes. Should I add my own check? IResource.createMarker(marker ID) doesn't have any checks.
| createMarker(resource, entry.getKey(), entry.getValue(), | ||
| markerId, IMarker.SEVERITY_ERROR); | ||
| BannedElement element = entry.getKey(); | ||
| createMarker(resource, element, entry.getValue(), |
There was a problem hiding this comment.
You don't need to pass the markerId and severity separately. createMarker can simply call the getter methods on element
| @@ -42,9 +42,7 @@ protected void validate(IResource resource, byte[] bytes) | |||
| Map<BannedElement, Integer> bannedElementOffsetMap = | |||
There was a problem hiding this comment.
separate issue, but I just realized that since we're using BannedElement as a key in a map, we should give it equals and hashCode methods: #1507
| int elementOffset, String markerId, int severity) throws CoreException { | ||
| IMarker marker = resource.createMarker(markerId); | ||
| marker.setAttribute(IMarker.SEVERITY, severity); | ||
| static void createMarker(IResource resource, BannedElement element, int elementOffset) |
There was a problem hiding this comment.
Do we need to pass elementOffset here? Is it different than
element.getStart().getLineNumber()?
There was a problem hiding this comment.
Yes, elementOffset is the character offset
Codecov Report
@@ Coverage Diff @@
## master #1506 +/- ##
===========================================
+ Coverage 70.09% 70.19% +0.1%
- Complexity 1263 1272 +9
===========================================
Files 222 225 +3
Lines 8860 8887 +27
Branches 751 755 +4
===========================================
+ Hits 6210 6238 +28
+ Misses 2334 2332 -2
- Partials 316 317 +1
Continue to review full report at Codecov.
|
5 participants
Creates subclasses of BannedElement that contain a marker ID and severity, to allow validators to add more than one type of marker to a resource during validation.
@elharo @meltsufin