Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add missing scanning of CtExecutableReference's parameters in CtScanner #601

Merged
merged 1 commit into from
Apr 22, 2016

Conversation

leventov
Copy link
Contributor

Sorry I'm lazy to add a test for this because this is obviously a missing AST scanning branch, the test is going to be trivial from the source: "yes, we actually didn't visit execRef's parameters".

@monperrus
Copy link
Collaborator

It's indeed obvious. However, a test is not for now, it's for the future. If somebody in one year makes a change (in this class or elsewhere) that breaks this expected behavior, s/he will notice it.

so I kindly ask you to add a test :-)

@monperrus monperrus changed the title Add missing scanning of CtExecutionReference's parameters in CtScanner Add missing scanning of CtExecutableReference's parameters in CtScanner Apr 19, 2016
@leventov
Copy link
Contributor Author

Down the rabbit hole...

To write a test I decided to improve hashCodeVisitor, because currently it is way too simple, emitting just 1 for a lot of elements (all which don't have names). Here is my version:

/**
 * Copyright (C) 2006-2015 INRIA and contributors
 * Spoon - http://spoon.gforge.inria.fr/
 *
 * This software is governed by the CeCILL-C License under French law and
 * abiding by the rules of distribution of free software. You can use, modify
 * and/or redistribute the software under the terms of the CeCILL-C license as
 * circulated by CEA, CNRS and INRIA at http://www.cecill.info.
 *
 * This program is distributed in the hope that it will be useful, but WITHOUT
 * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
 * FITNESS FOR A PARTICULAR PURPOSE. See the CeCILL-C License for more details.
 *
 * The fact that you are presently reading this means that you have had
 * knowledge of the CeCILL-C license and that you accept its terms.
 */
package spoon.support.visitor;

import spoon.reflect.declaration.CtElement;
import spoon.reflect.declaration.CtNamedElement;
import spoon.reflect.visitor.CtScanner;

import java.util.Collection;
import java.util.Iterator;
import java.util.List;
import java.util.Set;

/** Responsible for computing CtElement.hashCode().
 * Version that is fast and compatible with EqualVisitor
 */
public class HashcodeVisitor extends CtScanner {

    /**
     * This prime is used in google/auto
     */
    private static final int HASH_CODE_X = 1000003;

    private boolean goDown = true;
    private int hashCode = 1;

    @Override
    protected void enter(CtElement e) {
        goDown = false;
    }

    @Override
    protected void exit(CtElement e) {
        goDown = true;
    }

    @Override
    public void scan(CtElement element) {
        hashCode *= HASH_CODE_X;
        if (element instanceof CtNamedElement) {
            hashCode += ((CtNamedElement) element).getSimpleName().hashCode();
        } else {
            hashCode += 1;
        }
        if (goDown) {
            super.scan(element);
        }
    }

    @Override
    public void scan(Object o) {
        if (o instanceof CtElement) {
            scan((CtElement) o);
        }
        if (o instanceof Collection<?>) {
            Iterator<?> it = ((Collection) o).iterator();
            if (it.hasNext() && it.next() instanceof CtElement) {
                scan((Collection<? extends CtElement>) o);
            }
        }
    }

    @Override
    public void scan(Collection<? extends CtElement> elements) {
        hashCode *= HASH_CODE_X;
        int result = 1;
        if (elements instanceof List) {
            for (CtElement element : elements) {
                result *= HASH_CODE_X;
                if (element instanceof CtNamedElement) {
                    result += ((CtNamedElement) element).getSimpleName().hashCode();
                } else {
                    result += 1;
                }
            }
        } else if (elements instanceof Set) {
            for (CtElement element : elements) {
                if (element instanceof CtNamedElement) {
                    result ^= ((CtNamedElement) element).getSimpleName().hashCode();
                } else {
                    result ^= 1;
                }
            }
        }
        // if elements is not List and not Set, result variable stays 1 because we cannot
        // compute hash code of unknown type of collection
        hashCode += result;
    }

    public int getHasCode() {
        return hashCode;
    }
}

But when I tried to test it, it turned out that equals is very often considers elements equal while they actually are not...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants