eclipse / Xpect

This repository has been rewritten to move to the Eclipse Foundation. Find the old history here: https://github.com/TypeFox/Xpect
http://www.xpect-tests.org/
Eclipse Public License 2.0
32 stars 28 forks source link

ContentTypeUtil should not throw RuntimeExceptions #168

Closed iloveeclipse closed 9 years ago

iloveeclipse commented 9 years ago

We observe test failures with xtext involved. The root cause is that the ContentTypeUtil throws RuntimeExceptions which are propagated to the caller which should not throw any exception.

Unexpected log from plugin: org.eclipse.core.runtime, message: 'Exception in Decorator. The 'File Icons Based On Content Analysis' decorator will be disabled.'
Unexpected log from plugin: org.eclipse.core.runtime, message: 'org.eclipse.core.runtime.CoreException: File not found: /xxx/lock_file.'
org.eclipse.core.runtime.CoreException: File not found: /xxx/lock_file.
    at org.eclipse.core.internal.filesystem.Policy.error(Policy.java:55)
    at org.eclipse.core.internal.filesystem.local.LocalFile.openInputStream(LocalFile.java:377)
    at org.eclipse.core.internal.localstore.FileSystemResourceManager.read(FileSystemResourceManager.java:797)
    at org.eclipse.core.internal.resources.File.getContents(File.java:289)
    at org.eclipse.core.internal.resources.File.getContents(File.java:278)
    at org.xpect.ui.util.ContentTypeUtil.getContentType(ContentTypeUtil.java:36)
    at org.xpect.ui.editor.XpectEditorAssociationOverride.overrideDefaultEditor(XpectEditorAssociationOverride.java:76)
    at org.eclipse.ui.ide.IDE.overrideDefaultEditorAssociation(IDE.java:917)
    at org.eclipse.ui.ide.IDE.getDefaultEditor(IDE.java:1465)
    at org.eclipse.ui.ide.IDE.getDefaultEditor(IDE.java:1416)
    at org.eclipse.ui.internal.ide.ContentTypeDecorator.decorate(ContentTypeDecorator.java:51)
    at org.eclipse.ui.internal.decorators.LightweightDecoratorDefinition.decorate(LightweightDecoratorDefinition.java:269)
    at org.eclipse.ui.internal.decorators.LightweightDecoratorManager$LightweightRunnable.run(LightweightDecoratorManager.java:81)
    at org.eclipse.core.runtime.SafeRunner.run(SafeRunner.java:42)
    at org.eclipse.ui.internal.decorators.LightweightDecoratorManager.decorate(LightweightDecoratorManager.java:365)
    at org.eclipse.ui.internal.decorators.LightweightDecoratorManager.getDecorations(LightweightDecoratorManager.java:347)
    at org.eclipse.ui.internal.decorators.DecorationScheduler$1.ensureResultCached(DecorationScheduler.java:370)
    at org.eclipse.ui.internal.decorators.DecorationScheduler$1.run(DecorationScheduler.java:330)
    at org.eclipse.core.internal.jobs.Worker.run(Worker.java:54)
Caused by: java.io.FileNotFoundException: /xxx/lock_file (No such file or directory)
    at java.io.FileInputStream.open(Native Method)
    at java.io.FileInputStream.<init>(FileInputStream.java:146)
    at org.eclipse.core.internal.filesystem.local.LocalFile.openInputStream(LocalFile.java:368)
    ... 17 more

The code in IDE.overrideDefaultEditorAssociation() do notr expect any runtime exceptions - it is designed in the way that if there is no match for whatever reason, nothing "bad" should happen.

To fix this, the exceptions should be avoided and a "binary" type should be returned.

/*******************************************************************************
 * Copyright (c) 2012 itemis AG (http://www.itemis.eu) and others.
 * All rights reserved. This program and the accompanying materials
 * are made available under the terms of the Eclipse Public License v1.0
 * which accompanies this distribution, and is available at
 * http://www.eclipse.org/legal/epl-v10.html
 *******************************************************************************/
package org.xpect.ui.util;

import java.io.IOException;
import java.io.InputStreamReader;
import java.io.Reader;

import org.eclipse.core.resources.IFile;
import org.eclipse.core.runtime.CoreException;
import org.xpect.XpectConstants;

/**
 * @author Moritz Eysholdt - Initial contribution and API
 */
public class ContentTypeUtil {

    private static final String XPECT_SETUP = "XPECT_SETUP";

    public enum XpectContentType {
        BINARY, TEXT, XPECT
    }

    public XpectContentType getContentType(IFile file) {
        if (file == null || !file.exists())
            return XpectContentType.BINARY;
        if (XpectConstants.XT_FILE_EXT.equals(file.getFileExtension()))
            return XpectContentType.XPECT;
        Reader contents = null;
        try {
            contents = new InputStreamReader(file.getContents(), file.getCharset());
            char[] buf = new char[1024];
            int len = contents.read(buf);
            for (int i = 0; i < len; i++) {
                char c = buf[i];
                if (c < ' ' && c != '\n' && c != '\r' && c != '\t')
                    return XpectContentType.BINARY;
            }
            String stringBuf = new String(buf);
            int index = stringBuf.indexOf(XPECT_SETUP);
            if (index >= 0 && index < stringBuf.length() + XPECT_SETUP.length()) {
                if (Character.isWhitespace(stringBuf.charAt(index + XPECT_SETUP.length())))
                    return XpectContentType.XPECT;
            }
            return XpectContentType.TEXT;
        } catch (CoreException e) {
            return XpectContentType.BINARY;
        } catch (IOException e) {
            return XpectContentType.BINARY;
        } finally {
            if (contents != null)
                try {
                    contents.close();
                } catch (IOException e) {
                    return XpectContentType.BINARY;
                }
        }

    }
}
iloveeclipse commented 9 years ago

Moritz, there is no need to log the error since it is expected that the test might fail. So could you please remove new logging you've added with b3bfc60 - it will pollute the error log exact in the same way as before, with a slightly different stack trace.

The point of this method is to check if the given resource (of any kind and in any state) matches the xpect content type. If the resource is in bad state, it is just supposed to return "BINARY".