Skip to content

Commit

Permalink
refix: quick-fix for extraneous empty line(s) in manifest editor
Browse files Browse the repository at this point in the history
Extraneous lines are errneous to the manifest compiler,
but it does not recommend / provide a resolution. Add a
quick fix to remove extraneous line. Takes care of the
empty valid empty lines at the end of manifests.

In the main iteration loop, identify empty lines and push
into a list. At the end of the loop, aggregate all the
empty lines into one single marker. This way, we address
issues that can arise from race conditions when two threas
try to manipulate the document context simultaneously.

Fixes: #563
  • Loading branch information
gireeshpunathil authored and jukzi committed Jul 10, 2023
1 parent c8754be commit b4bd5e2
Show file tree
Hide file tree
Showing 6 changed files with 124 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2017 IBM Corporation and others.
* Copyright (c) 2000, 2023 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand All @@ -15,9 +15,11 @@

import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;

import java.util.StringJoiner;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.runtime.IProgressMonitor;
import org.eclipse.core.runtime.OperationCanceledException;
Expand Down Expand Up @@ -121,6 +123,9 @@ protected void parseManifest(IDocument document, IProgressMonitor monitor) {
fHeaders = new HashMap<>();
JarManifestHeader header = null;
int l = 0;
ArrayList<Integer> emptyLineCandidates = new ArrayList<>();
ArrayList<Integer> emptyLines = new ArrayList<>();
int numLines = document.getNumberOfLines();
for (; l < document.getNumberOfLines(); l++) {
if (l % 100 == 0) {
checkCanceled(monitor);
Expand All @@ -140,10 +145,15 @@ protected void parseManifest(IDocument document, IProgressMonitor monitor) {
}
// parse
if (line.length() == 0) {
// Empty Line
// Empty Line in the beginning
if (l == 0) {
report(PDECoreMessages.BundleErrorReporter_noMainSection, 1, CompilerFlags.ERROR, PDEMarkerFactory.CAT_FATAL);
return;

// empty line elsewhere
} else if (l != numLines - 1) {
emptyLineCandidates.add(Integer.valueOf(l));
continue;
}
/* flush last line */
if (header != null) {
Expand All @@ -152,6 +162,10 @@ protected void parseManifest(IDocument document, IProgressMonitor monitor) {
}
break; /* done processing main attributes */
}

emptyLines.addAll(emptyLineCandidates);
emptyLineCandidates.clear();

if (line.charAt(0) == ' ') {
// Continuation Line
if (l == 0) { /* if no previous line */
Expand Down Expand Up @@ -194,6 +208,27 @@ protected void parseManifest(IDocument document, IProgressMonitor monitor) {
}

}
if (emptyLines.size() > 0) {
// reverse the list to aid the quick-fix resolver
// to remove the line starting from the fag end,
// saving additional calculations to adjust the
// line numbers after each removal in-line.
Collections.reverse(emptyLines);

// build a marker with all the line info aggregated into it,
// with the first empty line as the anchor line.
StringJoiner str = new StringJoiner(","); //$NON-NLS-1$
int start = emptyLines.get(emptyLines.size() - 1);
for (Integer i : emptyLines) {
str.add(i.toString());
}
VirtualMarker marker = report(PDECoreMessages.BundleErrorReporter_noNameHeader, start + 1,
CompilerFlags.ERROR, PDEMarkerFactory.M_EXTRANEOUS_EMPTY_LINES, PDEMarkerFactory.CAT_FATAL);
if (marker != null) {
marker.setAttribute(PDEMarkerFactory.EMPTY_LINE, str.toString());
}
}

if (header != null) {
// lingering header, line not terminated
VirtualMarker marker = report(PDECoreMessages.BundleErrorReporter_noLineTermination, l, CompilerFlags.ERROR, PDEMarkerFactory.M_NO_LINE_TERMINATION, PDEMarkerFactory.CAT_FATAL);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2000, 2021 IBM Corporation and others.
* Copyright (c) 2000, 2023 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -76,6 +76,7 @@ public class PDEMarkerFactory {
public static final int M_EXEC_ENV_TOO_LOW = 0x1029; // other problem
public static final int M_CONFLICTING_AUTOMATIC_MODULE = 0x1030; // other
// problem
public static final int M_EXTRANEOUS_EMPTY_LINES = 0X1031; // fatal problem

// build properties fixes
public static final int B_APPEND_SLASH_FOLDER_ENTRY = 0x2001;
Expand All @@ -99,6 +100,7 @@ public class PDEMarkerFactory {
public static final String ATTR_CAN_ADD = "deprecatedAutostart.canAdd"; //$NON-NLS-1$
public static final String ATTR_HEADER = "deprecatedAutostart.header"; //$NON-NLS-1$
public static final String REQUIRED_EXEC_ENV = "executionEnvironment.key"; //$NON-NLS-1$
public static final String EMPTY_LINE = "emptyLine"; //$NON-NLS-1$
/**
* Boolean attribute for marker added when no newline is found at the end of a manifest. Value is
* <code>true</code> if there is character content on the last line that should be
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2617,6 +2617,10 @@ public class PDEUIMessages extends NLS {

public static String NoLineTerminationResolutionRemove_label;

public static String ExtraneousLineResolutionRemove_description;

public static String ExtraneousLineResolutionRemove_label;

public static String OptionalImportPkgResolution_description;

public static String OptionalImportPkgResolution_label;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/*******************************************************************************
* Copyright (c) 2023 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* IBM Corporation - initial API and implementation
* Gireesh Punathil <[email protected]> Initial implementation
*******************************************************************************/

package org.eclipse.pde.internal.ui.correction;

import java.util.StringTokenizer;
import org.eclipse.core.resources.IMarker;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.jface.text.*;
import org.eclipse.pde.internal.core.builders.PDEMarkerFactory;
import org.eclipse.pde.internal.core.text.bundle.BundleModel;
import org.eclipse.pde.internal.ui.PDEPlugin;
import org.eclipse.pde.internal.ui.PDEUIMessages;

/**
* <p>Represents a resolution to the problem of the bundle manifest
* having extraneous lines.</p>
*/
public class ExtraneousLinesResolution extends AbstractManifestMarkerResolution {
/**
* Creates a new resolution
*
* @param type
* {@link AbstractPDEMarkerResolution#REMOVE_TYPE} to delete
* lines
*/
public ExtraneousLinesResolution(int type, IMarker marker) {
super(type, marker);
}

/**
* Resolves the problem by extracting the empty line and removing it.
*/
@Override
protected void createChange(BundleModel model) {
IDocument doc = model.getDocument();
int line = 0;
try {
String emptyLines = (String) marker.getAttribute(PDEMarkerFactory.EMPTY_LINE);
StringTokenizer st = new StringTokenizer(emptyLines, ","); //$NON-NLS-1$
while (st.hasMoreElements()) {
line = Integer.parseInt(st.nextToken());
IRegion l = doc.getLineInformation(line);
doc.replace(l.getOffset(), l.getLength() + 1, ""); //$NON-NLS-1$
}
} catch (BadLocationException | CoreException | NumberFormatException e) {
PDEPlugin.log(e);
}
}

@Override
public String getDescription() {
return PDEUIMessages.ExtraneousLineResolutionRemove_description;
}

@Override
public String getLabel() {
return PDEUIMessages.ExtraneousLineResolutionRemove_label;
}

}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*******************************************************************************
* Copyright (c) 2005, 2022 IBM Corporation and others.
* Copyright (c) 2005, 2023 IBM Corporation and others.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
Expand Down Expand Up @@ -130,6 +130,9 @@ public IMarkerResolution[] getNonConfigSevResolutions(IMarker marker) {
return new IMarkerResolution[] {new AddBundleClassPathMarkerResolution(AbstractPDEMarkerResolution.CREATE_TYPE, marker)};
case PDEMarkerFactory.M_LAZYLOADING_HAS_NO_EFFECT :
return new IMarkerResolution[] {new RemoveLazyLoadingDirectiveResolution(AbstractPDEMarkerResolution.REMOVE_TYPE, marker.getAttribute("header", ICoreConstants.ECLIPSE_LAZYSTART), marker)}; //$NON-NLS-1$
case PDEMarkerFactory.M_EXTRANEOUS_EMPTY_LINES:
return new IMarkerResolution[] {
new ExtraneousLinesResolution(AbstractPDEMarkerResolution.REMOVE_TYPE, marker) };
case PDEMarkerFactory.M_NO_LINE_TERMINATION :
if (marker.getAttribute(PDEMarkerFactory.ATTR_HAS_CONTENT, true)) {
return new IMarkerResolution[] {new NoLineTerminationResolution(AbstractPDEMarkerResolution.CREATE_TYPE,marker)};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2353,6 +2353,8 @@ NoLineTerminationResolutionCreate_description=Adds a line break at the end of th
NoLineTerminationResolutionCreate_label=Add a line break after the header
NoLineTerminationResolutionRemove_description=Removes all whitespace characters from the last line of the manifest
NoLineTerminationResolutionRemove_label=Remove excess whitespace from the end of the manifest
ExtraneousLineResolutionRemove_label=Remove extraneous lines
ExtraneousLineResolutionRemove_description=Removes extraneous lines in the manifest header
OpenManifestsAction_title=Open Manifest
OpenPluginManifestsAction_title=Plug-in Manifest Editor Error
OpenManifestsAction_cannotFind=Cannot find manifest for {0}.
Expand Down

0 comments on commit b4bd5e2

Please sign in to comment.