Is having a return statement just to satisfy syntax bad practice?

Consider the following code:

public Object getClone(Cloneable a) throws TotallyFooException {

    if (a == null) {
        throw new TotallyFooException();
    }
    else {
        try {
            return a.clone();
        } catch (CloneNotSupportedException e) {
            e.printStackTrace();
        }
    }
    //cant be reached, in for syntax
    return null;
}

The return null; is necessary since an exception may be caught, however in such a case since we already checked if it was null (and lets assume we know the class we are calling supports cloning) so we know the try statement will never fail.

Is it bad practice to put in the extra return statement at the end just to satisfy the syntax and avoid compile errors (with a comment explaining it will not be reached), or is there a better way to code something like this so that the extra return statement is unnecessary?

A clearer way without an extra return statement is as follows. I wouldn't catch CloneNotSupportedException either, but let it go to the caller.

if (a != null) {
    try {
        return a.clone();
    } catch (CloneNotSupportedException e) {
        e.printStackTrace();
    }
}
throw new TotallyFooException();

It's almost always possible to fiddle with the order to end up with a more straight-forward syntax than what you initially have.

It definitely can be reached. Note that you're only printing the stacktrace in the catch clause.

In the scenario where a != null and there will be an exception, the return null will be reached. You can remove that statement and replace it with throw new TotallyFooException();.

In general^*, if null is a valid result of a method (i.e. the user expects it and it means something) then returning it as a signal for "data not found" or exception happened is not a good idea. Otherwise, I don't see any problem why you shouldn't return null.

Take for example the Scanner#ioException method:

Returns the IOException last thrown by this Scanner's underlying Readable. This method returns null if no such exception exists.

In this case, the returned value null has a clear meaning, when I use the method I can be sure that I got null only because there was no such exception and not because the method tried to do something and it failed.

^*Note that sometimes you do want to return null even when the meaning is ambiguous. For example the HashMap#get:

A return value of null does not necessarily indicate that the map contains no mapping for the key; it's also possible that the map explicitly maps the key to null. The containsKey operation may be used to distinguish these two cases.

In this case null can indicate that the value null was found and returned, or that the hashmap doesn't contain the requested key.

Is it bad practice to put in the extra return statement at the end just to satisfy the syntax and avoid compile errors (with a comment explaining it will not be reached)

I think return null is bad practice for the terminus of an unreachable branch. It is better to throw a RuntimeException (AssertionError would also be acceptable) as to get to that line something has gone very wrong and the application is in an unknown state. Most like this is (like above) because the developer has missed something (Objects can be none-null and un-cloneable).

I'd likely not use InternalError unless I'm very very sure that the code is unreachable (for example after a System.exit()) as it is more likely that I make a mistake than the VM.

I'd only use a custom exception (such as TotallyFooException) if getting to that "unreachable line" means the same thing as anywhere else you throw that exception.

You caught the CloneNotSupportedException which means your code can handle it. But after you catch it, you have literally no idea what to do when you reach the end of the function, which implies that you couldn't handle it. So you're right that it is a code smell in this case, and in my view means you should not have caught CloneNotSupportedException.

I would prefer to use Objects.requireNonNull() to check if the Parameter a is not null. So it's clear when you read the code that the parameter should not be null.

And to avoid checked Exceptions I would re throw the CloneNotSupportedException as a RuntimeException.

For both you could add nice text with the intention why this shouldn't happen or be the case.

public Object getClone(Object a) {

    Objects.requireNonNull(a);

    try {
        return a.clone();
    } catch (CloneNotSupportedException e) {
        throw new IllegalArgumentException(e);
    }

}