Skip to content

Commit c06f361

Browse files
vogellaclaude
authored andcommitted
Fix incorrect SubMonitor usage patterns
This commit addresses several SubMonitor anti-patterns identified throughout the codebase, based on best practices from the Eclipse article on Progress Monitors. Changes made: 1. Remove unnecessary null checks before SubMonitor.convert() - SubMonitor.convert() can handle null monitors - Fixed in team Policy.java files (core and ui) 2. Remove unnecessary SubMonitor.done() calls - SubMonitor.done() is a no-op and should not be called - Removed 27 instances across resources and team bundles: * org.eclipse.core.filesystem: LocalFile.java (2 calls) * org.eclipse.core.resources: File.java (6), Folder.java (1), Project.java (3), SaveManager.java (4), Workspace.java (7) * org.eclipse.compare.core: RangeComparatorLCS.java (1) * org.eclipse.compare: AddFromHistoryAction.java (1) * org.eclipse.team.core: BundleImporterDelegate.java (1), Subscriber.java (1) 3. Remove redundant cancellation checks before split() - SubMonitor.split() already checks for cancellation - Fixed in AutoBuildJob.java Reference: https://www.eclipse.org/articles/Article-Progress-Monitors/article.html
1 parent 3e8e40e commit c06f361

File tree

13 files changed

+2
-51
lines changed

13 files changed

+2
-51
lines changed

resources/bundles/org.eclipse.core.filesystem/src/org/eclipse/core/internal/filesystem/local/LocalFile.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -179,8 +179,6 @@ protected void copyFile(IFileInfo sourceInfo, IFileStore destination, int option
179179
Policy.error(EFS.ERROR_EXISTS, NLS.bind(Messages.fileExists, target.filePath), e);
180180
} catch (IOException e) {
181181
Policy.error(EFS.ERROR_WRITE, NLS.bind(Messages.failedCopy, this.filePath, target.filePath), e);
182-
} finally {
183-
subMonitor.done();
184182
}
185183
} else {
186184
super.copyFile(sourceInfo, destination, options, monitor);
@@ -503,8 +501,6 @@ public void move(IFileStore destFile, int options, IProgressMonitor monitor) thr
503501
}
504502
// fall back to default implementation
505503
super.move(destFile, options, subMonitor.newChild(1));
506-
} finally {
507-
subMonitor.done();
508504
}
509505
}
510506

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/events/AutoBuildJob.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ public void preferenceChange(PreferenceChangeEvent event) {
297297
public IStatus run(IProgressMonitor monitor) {
298298
SubMonitor subMonitor = SubMonitor.convert(monitor, 1);
299299
synchronized (this) {
300-
if (subMonitor.isCanceled() || isInterrupted()) {
300+
if (isInterrupted()) {
301301
return canceled();
302302
}
303303
}

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/File.java

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,6 @@ public void appendContents(InputStream content, int updateFlags, IProgressMonito
9191
}
9292
} catch (IOException streamCloseIgnored) {
9393
// ignore;
94-
} finally {
95-
subMonitor.done();
9694
}
9795
}
9896

@@ -163,8 +161,6 @@ public void create(InputStream content, int updateFlags, IProgressMonitor monito
163161
}
164162
} catch (IOException streamCloseIgnored) {
165163
// ignore;
166-
} finally {
167-
subMonitor.done();
168164
}
169165
}
170166

@@ -207,8 +203,6 @@ public void create(byte[] content, int updateFlags, IProgressMonitor monitor) th
207203
} finally {
208204
workspace.endOperation(rule, true);
209205
}
210-
} finally {
211-
subMonitor.done();
212206
}
213207
}
214208

@@ -244,7 +238,6 @@ IFileInfo create(int updateFlags, IProgressMonitor subMonitor, IFileStore store)
244238
throw new ResourceException(IResourceStatus.FAILED_WRITE_LOCAL, getFullPath(), message, null);
245239
}
246240
}
247-
subMonitor.done();
248241

249242
workspace.createResource(this, updateFlags);
250243
return localInfo;
@@ -510,8 +503,6 @@ public void setContents(InputStream content, int updateFlags, IProgressMonitor m
510503
}
511504
} catch (IOException streamCloseIgnored) {
512505
// ignore;
513-
} finally {
514-
subMonitor.done();
515506
}
516507
}
517508
@Override
@@ -541,8 +532,6 @@ public void setContents(byte[] content, int updateFlags, IProgressMonitor monito
541532
} finally {
542533
workspace.endOperation(rule, true);
543534
}
544-
} finally {
545-
subMonitor.done();
546535
}
547536
}
548537

@@ -616,7 +605,6 @@ public void setCharset(String newCharset, IProgressMonitor monitor) throws CoreE
616605
workspace.getWorkManager().operationCanceled();
617606
throw e;
618607
} finally {
619-
subMonitor.done();
620608
workspace.endOperation(rule, true);
621609
}
622610
}

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Folder.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,6 @@ public void create(int updateFlags, boolean local, IProgressMonitor monitor) thr
118118
workspace.getWorkManager().operationCanceled();
119119
throw e;
120120
} finally {
121-
subMonitor.done();
122121
workspace.endOperation(rule, true);
123122
}
124123
}

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Project.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,6 @@ public void close(IProgressMonitor monitor) throws CoreException {
252252
workspace.getWorkManager().operationCanceled();
253253
throw e;
254254
} finally {
255-
subMonitor.done();
256255
workspace.endOperation(rule, true);
257256
}
258257
}
@@ -360,7 +359,6 @@ public void create(IProjectDescription description, int updateFlags, IProgressMo
360359
workspace.getWorkManager().operationCanceled();
361360
throw e;
362361
} finally {
363-
subMonitor.done();
364362
workspace.endOperation(rule, true);
365363
}
366364
}
@@ -647,8 +645,6 @@ public void run(IProgressMonitor innerMonitor) throws CoreException {
647645
workspace.endOperation(notificationsRule, false);
648646
}
649647
}
650-
} finally {
651-
subMonitor.done();
652648
}
653649
}
654650

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/SaveManager.java

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,8 +274,6 @@ public void run() throws Exception {
274274
SafeRunner.run(code);
275275
subMonitor.worked(1);
276276
}
277-
} finally {
278-
subMonitor.done();
279277
}
280278
}
281279

@@ -1603,8 +1601,6 @@ protected void snapTree(ElementTree tree, IProgressMonitor monitor) throws CoreE
16031601
if (Policy.DEBUG_SAVE_TREE) {
16041602
Policy.debug("Snapshot Workspace Tree: " + (System.currentTimeMillis() - start) + "ms"); //$NON-NLS-1$ //$NON-NLS-2$
16051603
}
1606-
} finally {
1607-
subMonitor.done();
16081604
}
16091605
}
16101606

@@ -2181,7 +2177,6 @@ protected void writeTree(Map<String, ElementTree> statesToSave, DataOutputStream
21812177
output.writeUTF(string);
21822178
}
21832179
} finally {
2184-
subMonitor.done();
21852180
if (!wasImmutable) {
21862181
workspace.newWorkingTree();
21872182
}
@@ -2257,7 +2252,6 @@ protected void writeTree(Project project, DataOutputStream output, IProgressMoni
22572252
output.writeUTF(string);
22582253
}
22592254
} finally {
2260-
subMonitor.done();
22612255
if (!wasImmutable) {
22622256
workspace.newWorkingTree();
22632257
}

resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Workspace.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -632,7 +632,6 @@ private void buildInternal(IBuildConfiguration[] requestedConfigs, int trigger,
632632
throw new ResourceException(result);
633633
}
634634
} finally {
635-
subMonitor.done();
636635
// building may close the tree, but we are still inside an operation so open it
637636
if (tree.isImmutable()) {
638637
newWorkingTree();
@@ -1135,7 +1134,6 @@ public IStatus copy(IResource[] resources, IPath destination, int updateFlags, I
11351134
getWorkManager().operationCanceled();
11361135
throw e;
11371136
} finally {
1138-
subMonitor.done();
11391137
endOperation(getRoot(), true);
11401138
}
11411139
if (status.matches(IStatus.ERROR)) {
@@ -1529,7 +1527,6 @@ public IStatus delete(IResource[] resources, int updateFlags, IProgressMonitor m
15291527
getWorkManager().operationCanceled();
15301528
throw e;
15311529
} finally {
1532-
subMonitor.done();
15331530
endOperation(getRoot(), true);
15341531
}
15351532
}
@@ -2206,7 +2203,6 @@ public IStatus move(IResource[] resources, IPath destination, int updateFlags, I
22062203
getWorkManager().operationCanceled();
22072204
throw e;
22082205
} finally {
2209-
subMonitor.done();
22102206
endOperation(getRoot(), true);
22112207
}
22122208
if (status.matches(IStatus.ERROR)) {
@@ -2512,7 +2508,6 @@ public void run(ICoreRunnable action, ISchedulingRule rule, int options, IProgre
25122508
}
25132509
throw e;
25142510
} finally {
2515-
subMonitor.done();
25162511
if (avoidNotification) {
25172512
notificationManager.endAvoidNotify();
25182513
}
@@ -2641,7 +2636,6 @@ protected void shutdown(IProgressMonitor monitor) throws CoreException {
26412636
throw new CoreException(status);
26422637
}
26432638
} finally {
2644-
subMonitor.done();
26452639
}
26462640
}
26472641

@@ -2941,8 +2935,6 @@ private void createMultiple(ConcurrentMap<File, byte[]> filesToCreate, int updat
29412935
} finally {
29422936
endOperation(rule, true);
29432937
}
2944-
} finally {
2945-
subMonitor.done();
29462938
}
29472939
}
29482940
}

team/bundles/org.eclipse.compare.core/src/org/eclipse/compare/rangedifferencer/RangeComparatorLCS.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,6 @@ public RangeDifference[] getDifferences(SubMonitor subMonitor, AbstractRangeDiff
135135

136136
}
137137
return differences.toArray(new RangeDifference[differences.size()]);
138-
} finally {
139-
subMonitor.done();
140138
}
141139
}
142140

team/bundles/org.eclipse.compare/compare/org/eclipse/compare/internal/AddFromHistoryAction.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,11 +130,7 @@ public void execute(IProgressMonitor pm) throws InvocationTargetException {
130130
IFileState fileState = s.fFileState;
131131
createContainers(file);
132132
SubMonitor subMonitor= SubMonitor.convert(pm, 1);
133-
try {
134-
file.create(fileState.getContents(), false, subMonitor);
135-
} finally {
136-
subMonitor.done();
137-
}
133+
file.create(fileState.getContents(), false, subMonitor);
138134
}
139135
} catch (CoreException e) {
140136
throw new InvocationTargetException(e);

team/bundles/org.eclipse.team.core/src/org/eclipse/team/core/importing/provisional/BundleImporterDelegate.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@ public IProject[] performImport(ScmUrlImportDescription[] descriptions, IProgres
103103
if (!references.isEmpty()) {
104104
SubMonitor subMonitor = SubMonitor.convert(monitor, references.size());
105105
result = psfCapability.addToWorkspace(references.toArray(new String[references.size()]), new ProjectSetSerializationContext(), subMonitor);
106-
subMonitor.done();
107106
}
108107
}
109108
return result;

0 commit comments

Comments
 (0)